From 242bc0e3bbc9a5f18319a8aabed9d6508bff3fce Mon Sep 17 00:00:00 2001 From: woodser Date: Tue, 25 Jul 2023 08:21:59 -0400 Subject: [PATCH] subtract mining fees from destinations in trade protocol fixes to scheduling and the deposit view display address usage context fix npe when price is null --- .../haveno/core/api/CoreDisputesService.java | 3 + .../haveno/core/api/CoreOffersService.java | 3 +- .../java/haveno/core/app/WalletAppSetup.java | 2 +- .../java/haveno/core/offer/OpenOffer.java | 4 + .../haveno/core/offer/OpenOfferManager.java | 120 ++++---- .../tasks/MakerReserveOfferFunds.java | 5 +- .../core/support/dispute/DisputeManager.java | 37 +-- .../arbitration/ArbitrationManager.java | 3 - .../support/dispute/refund/RefundManager.java | 2 +- .../main/java/haveno/core/trade/Trade.java | 33 +-- .../java/haveno/core/trade/TradeManager.java | 7 +- .../ArbitratorProcessDepositRequest.java | 6 +- .../tasks/MaybeSendSignContractRequest.java | 12 +- .../trade/protocol/tasks/RemoveOffer.java | 2 +- .../tasks/SellerPublishDepositTx.java | 2 +- .../tasks/TakerReserveTradeFunds.java | 2 +- .../java/haveno/core/util/VolumeUtil.java | 2 +- .../core/xmr/wallet/BtcWalletService.java | 10 +- .../core/xmr/wallet/XmrWalletService.java | 262 ++++++++---------- .../resources/i18n/displayStrings.properties | 3 + .../haveno/desktop/main/MainViewModel.java | 2 +- .../main/funds/deposit/DepositListItem.java | 19 +- .../main/funds/deposit/DepositView.java | 8 +- .../openoffer/OpenOffersDataModel.java | 2 +- .../steps/buyer/BuyerStep4View.java | 2 +- 25 files changed, 273 insertions(+), 280 deletions(-) diff --git a/core/src/main/java/haveno/core/api/CoreDisputesService.java b/core/src/main/java/haveno/core/api/CoreDisputesService.java index ed4813e041..912cdaa4d2 100644 --- a/core/src/main/java/haveno/core/api/CoreDisputesService.java +++ b/core/src/main/java/haveno/core/api/CoreDisputesService.java @@ -237,6 +237,9 @@ public class CoreDisputesService { throw new RuntimeException("Winner payout is more than the trade wallet's balance"); } long loserAmount = tradeAmount.add(buyerSecurityDeposit).add(sellerSecurityDeposit).subtract(BigInteger.valueOf(customWinnerAmount)).longValueExact(); + if (loserAmount < 0) { + throw new RuntimeException("Loser payout cannot be negative"); + } disputeResult.setBuyerPayoutAmount(BigInteger.valueOf(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? customWinnerAmount : loserAmount)); disputeResult.setSellerPayoutAmount(BigInteger.valueOf(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? loserAmount : customWinnerAmount)); } diff --git a/core/src/main/java/haveno/core/api/CoreOffersService.java b/core/src/main/java/haveno/core/api/CoreOffersService.java index bfc087f870..553dc091bd 100644 --- a/core/src/main/java/haveno/core/api/CoreOffersService.java +++ b/core/src/main/java/haveno/core/api/CoreOffersService.java @@ -248,8 +248,9 @@ public class CoreOffersService { for (String keyImage : offer.getOfferPayload().getReserveTxKeyImages()) { if (!seenKeyImages.add(keyImage)) { for (Offer offer2 : offers) { + if (offer == offer2) continue; if (offer2.getOfferPayload().getReserveTxKeyImages().contains(keyImage)) { - log.warn("Key image {} belongs to multiple offers, removing offer {}", keyImage, offer2.getId()); + log.warn("Key image {} belongs to multiple offers, seen in offer {}", keyImage, offer2.getId()); duplicateFundedOffers.add(offer2); } } diff --git a/core/src/main/java/haveno/core/app/WalletAppSetup.java b/core/src/main/java/haveno/core/app/WalletAppSetup.java index cc8959ebd8..1dd7e683f1 100644 --- a/core/src/main/java/haveno/core/app/WalletAppSetup.java +++ b/core/src/main/java/haveno/core/app/WalletAppSetup.java @@ -214,7 +214,7 @@ public class WalletAppSetup { if (rejectedTxErrorMessageHandler != null) { rejectedTxErrorMessageHandler.accept(Res.get("popup.warning.openOffer.makerFeeTxRejected", openOffer.getId(), txId)); } - openOfferManager.removeOpenOffer(openOffer, () -> { + openOfferManager.cancelOpenOffer(openOffer, () -> { log.warn("We removed an open offer because the maker fee was rejected by the Bitcoin " + "network. OfferId={}, txId={}", openOffer.getShortId(), txId); }, log::warn); diff --git a/core/src/main/java/haveno/core/offer/OpenOffer.java b/core/src/main/java/haveno/core/offer/OpenOffer.java index 6725c7253e..5688365686 100644 --- a/core/src/main/java/haveno/core/offer/OpenOffer.java +++ b/core/src/main/java/haveno/core/offer/OpenOffer.java @@ -203,6 +203,10 @@ public final class OpenOffer implements Tradable { return state == State.SCHEDULED; } + public boolean isAvailable() { + return state == State.AVAILABLE; + } + public boolean isDeactivated() { return state == State.DEACTIVATED; } diff --git a/core/src/main/java/haveno/core/offer/OpenOfferManager.java b/core/src/main/java/haveno/core/offer/OpenOfferManager.java index 1af60a4157..a144c00ede 100644 --- a/core/src/main/java/haveno/core/offer/OpenOfferManager.java +++ b/core/src/main/java/haveno/core/offer/OpenOfferManager.java @@ -350,7 +350,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe int size = openOffers.size(); // Copy list as we remove in the loop List openOffersList = new ArrayList<>(openOffers); - openOffersList.forEach(openOffer -> removeOpenOffer(openOffer, () -> { + openOffersList.forEach(openOffer -> cancelOpenOffer(openOffer, () -> { }, errorMessage -> { log.warn("Error removing open offer: " + errorMessage); })); @@ -505,7 +505,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe resultHandler.handleResult(transaction); }, (errorMessage) -> { log.warn("Error processing unposted offer {}: {}", openOffer.getId(), errorMessage); - onRemoved(openOffer); + onCancelled(openOffer); offer.setErrorMessage(errorMessage); errorMessageHandler.handleErrorMessage(errorMessage); }); @@ -515,7 +515,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe public void removeOffer(Offer offer, ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { Optional openOfferOptional = getOpenOfferById(offer.getId()); if (openOfferOptional.isPresent()) { - removeOpenOffer(openOfferOptional.get(), resultHandler, errorMessageHandler); + cancelOpenOffer(openOfferOptional.get(), resultHandler, errorMessageHandler); } else { log.warn("Offer was not found in our list of open offers. We still try to remove it from the offerbook."); errorMessageHandler.handleErrorMessage("Offer was not found in our list of open offers. " + "We still try to remove it from the offerbook."); @@ -561,15 +561,15 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } } - public void removeOpenOffer(OpenOffer openOffer, + public void cancelOpenOffer(OpenOffer openOffer, ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { if (!offersToBeEdited.containsKey(openOffer.getId())) { if (openOffer.isDeactivated()) { - onRemoved(openOffer); + onCancelled(openOffer); } else { offerBookService.removeOffer(openOffer.getOffer().getOfferPayload(), - () -> onRemoved(openOffer), + () -> onCancelled(openOffer), errorMessageHandler); } } else { @@ -647,7 +647,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } // remove open offer which thaws its key images - private void onRemoved(@NotNull OpenOffer openOffer) { + private void onCancelled(@NotNull OpenOffer openOffer) { Offer offer = openOffer.getOffer(); if (offer.getOfferPayload().getReserveTxKeyImages() != null) { xmrWalletService.thawOutputs(offer.getOfferPayload().getReserveTxKeyImages()); @@ -667,7 +667,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe getOpenOfferById(offer.getId()).ifPresent(openOffer -> { removeOpenOffer(openOffer); openOffer.setState(OpenOffer.State.CLOSED); - xmrWalletService.resetAddressEntriesForOpenOffer(offer.getId()); + xmrWalletService.resetOfferFundingForOpenOffer(offer.getId()); offerBookService.removeOffer(openOffer.getOffer().getOfferPayload(), () -> log.info("Successfully removed offer {}", offer.getId()), log::error); @@ -780,7 +780,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe latch.countDown(); }, errorMessage -> { log.warn("Error processing unposted offer {}: {}", scheduledOffer.getId(), errorMessage); - onRemoved(scheduledOffer); + onCancelled(scheduledOffer); errorMessages.add(errorMessage); latch.countDown(); }); @@ -808,8 +808,8 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe // handle split output offer if (openOffer.isSplitOutput()) { - - // get tx to fund split output + + // find tx with exact input amount MoneroTxWallet splitOutputTx = findSplitOutputFundingTx(openOffers, openOffer); if (openOffer.getScheduledTxHashes() == null && splitOutputTx != null) { openOffer.setScheduledTxHashes(Arrays.asList(splitOutputTx.getHash())); @@ -818,27 +818,25 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe openOffer.setState(OpenOffer.State.SCHEDULED); } - // handle split output available - if (splitOutputTx != null && !splitOutputTx.isLocked()) { - signAndPostOffer(openOffer, true, resultHandler, errorMessageHandler); + // if not found, create tx to split exact output + if (splitOutputTx == null) { + splitOrSchedule(openOffers, openOffer, offerReserveAmount); + } else if (!splitOutputTx.isLocked()) { + + // otherwise sign and post offer if split output available + signAndPostOffer(openOffer, true, resultHandler, (errMsg) -> { + + // on error, create new tx to split output if offer subaddress does not have exact output + int offerSubaddress = xmrWalletService.getOrCreateAddressEntry(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING).getSubaddressIndex(); + if (!splitOutputTx.getOutgoingTransfer().getSubaddressIndices().equals(Arrays.asList(offerSubaddress))) { + log.warn("Splitting new output because spending existing output(s) failed for offer {}", openOffer.getId()); + splitOrSchedule(openOffers, openOffer, offerReserveAmount); + resultHandler.handleResult(null); + } else { + errorMessageHandler.handleErrorMessage(errMsg); + } + }); return; - } else if (splitOutputTx == null) { - - // handle sufficient available balance to split output - boolean sufficientAvailableBalance = xmrWalletService.getWallet().getUnlockedBalance(0).compareTo(offerReserveAmount) >= 0; - if (sufficientAvailableBalance) { - - // create and relay tx to split output - splitOutputTx = createAndRelaySplitOutputTx(openOffer); // TODO: confirm with user? - - // schedule txs - openOffer.setScheduledTxHashes(Arrays.asList(splitOutputTx.getHash())); - openOffer.setSplitOutputTxHash(splitOutputTx.getHash()); - openOffer.setScheduledAmount(offerReserveAmount.toString()); - openOffer.setState(OpenOffer.State.SCHEDULED); - } else if (openOffer.getScheduledTxHashes() == null) { - scheduleOfferWithEarliestTxs(openOffers, openOffer); - } } } else { @@ -862,44 +860,65 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe }).start(); } + private void splitOrSchedule(List openOffers, OpenOffer openOffer, BigInteger offerReserveAmount) { + + // handle sufficient available balance to split output + boolean sufficientAvailableBalance = xmrWalletService.getWallet().getUnlockedBalance(0).compareTo(offerReserveAmount) >= 0; + if (sufficientAvailableBalance) { + + // create and relay tx to split output + MoneroTxWallet splitOutputTx = createAndRelaySplitOutputTx(openOffer); + + // schedule txs + openOffer.setScheduledTxHashes(Arrays.asList(splitOutputTx.getHash())); + openOffer.setSplitOutputTxHash(splitOutputTx.getHash()); + openOffer.setScheduledAmount(offerReserveAmount.toString()); + openOffer.setState(OpenOffer.State.SCHEDULED); + } else if (openOffer.getScheduledTxHashes() == null) { + scheduleOfferWithEarliestTxs(openOffers, openOffer); + } + } + public boolean hasAvailableOutput(BigInteger amount) { - return findSplitOutputFundingTx(getOpenOffers(), amount, null) != null; + return findSplitOutputFundingTx(getOpenOffers(), null, amount, null) != null; } private MoneroTxWallet findSplitOutputFundingTx(List openOffers, OpenOffer openOffer) { - - // return split output tx if already assigned - if (openOffer.getSplitOutputTxHash() != null) { - return xmrWalletService.getWallet().getTx(openOffer.getSplitOutputTxHash()); - } - - // find tx with exact output XmrAddressEntry addressEntry = xmrWalletService.getOrCreateAddressEntry(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING); - return findSplitOutputFundingTx(openOffers, openOffer.getOffer().getReserveAmount(), addressEntry.getSubaddressIndex()); + return findSplitOutputFundingTx(openOffers, openOffer, openOffer.getOffer().getReserveAmount(), addressEntry.getSubaddressIndex()); } - private MoneroTxWallet findSplitOutputFundingTx(List openOffers, BigInteger reserveAmount, Integer subaddressIndex) { + private MoneroTxWallet findSplitOutputFundingTx(List openOffers, OpenOffer openOffer, BigInteger reserveAmount, Integer preferredSubaddressIndex) { List fundingTxs = new ArrayList<>(); MoneroTxWallet earliestUnscheduledTx = null; - if (subaddressIndex != null) { - // return earliest tx with exact confirmed output to fund offer's subaddress if available + // return earliest tx with exact confirmed output to given subaddress if available + if (preferredSubaddressIndex != null) { + + // get txs with exact output amount fundingTxs = xmrWalletService.getWallet().getTxs(new MoneroTxQuery() .setIsConfirmed(true) .setOutputQuery(new MoneroOutputQuery() .setAccountIndex(0) - .setSubaddressIndex(subaddressIndex) + .setSubaddressIndex(preferredSubaddressIndex) .setAmount(reserveAmount) .setIsSpent(false) .setIsFrozen(false))); + + // return earliest tx if available earliestUnscheduledTx = getEarliestUnscheduledTx(openOffers, fundingTxs); if (earliestUnscheduledTx != null) return earliestUnscheduledTx; } + // return split output tx if already assigned + if (openOffer != null && openOffer.getSplitOutputTxHash() != null) { + return xmrWalletService.getWallet().getTx(openOffer.getSplitOutputTxHash()); + } + // cache all transactions including from pool List allTxs = xmrWalletService.getWallet().getTxs(new MoneroTxQuery().setIncludeOutputs(true)); - if (subaddressIndex != null) { + if (preferredSubaddressIndex != null) { // return earliest tx with exact incoming transfer to fund offer's subaddress if available (since outputs are not available until confirmed) fundingTxs.clear(); @@ -907,7 +926,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe boolean hasExactTransfer = tx.getTransfers(new MoneroTransferQuery() .setIsIncoming(true) .setAccountIndex(0) - .setSubaddressIndex(subaddressIndex) + .setSubaddressIndex(preferredSubaddressIndex) .setAmount(reserveAmount)).size() > 0; if (hasExactTransfer) fundingTxs.add(tx); } @@ -982,13 +1001,16 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe private MoneroTxWallet createAndRelaySplitOutputTx(OpenOffer openOffer) { BigInteger reserveAmount = openOffer.getOffer().getReserveAmount(); - xmrWalletService.swapTradeEntryToAvailableEntry(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING); // change funding subaddress in case funded with unsuitable output // TODO: unecessary with destination funding - String fundingSubaddress = xmrWalletService.getNewAddressEntry(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING).getAddressString(); - return xmrWalletService.getWallet().createTx(new MoneroTxConfig() + xmrWalletService.swapAddressEntryToAvailable(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING); // change funding subaddress in case funded with unsuitable output(s) + String fundingSubaddress = xmrWalletService.getOrCreateAddressEntry(openOffer.getId(), XmrAddressEntry.Context.OFFER_FUNDING).getAddressString(); + log.info("Creating split output tx to fund offer {}", openOffer.getId()); + MoneroTxWallet splitOutputTx = xmrWalletService.getWallet().createTx(new MoneroTxConfig() .setAccountIndex(0) .setAddress(fundingSubaddress) .setAmount(reserveAmount) .setRelay(true)); + log.info("Done creating split output tx to fund offer {}", openOffer.getId()); + return splitOutputTx; } private BigInteger getScheduledAmount(List openOffers) { diff --git a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java index ea89213534..b6825679a6 100644 --- a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java +++ b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java @@ -54,10 +54,9 @@ public class MakerReserveOfferFunds extends Task { BigInteger sendAmount = offer.getDirection() == OfferDirection.BUY ? BigInteger.valueOf(0) : offer.getAmount(); BigInteger securityDeposit = offer.getDirection() == OfferDirection.BUY ? offer.getBuyerSecurityDeposit() : offer.getSellerSecurityDeposit(); String returnAddress = model.getXmrWalletService().getOrCreateAddressEntry(offer.getId(), XmrAddressEntry.Context.TRADE_PAYOUT).getAddressString(); - BigInteger exactOutputAmount = model.getOpenOffer().isSplitOutput() ? model.getOpenOffer().getOffer().getReserveAmount() : null; XmrAddressEntry fundingEntry = model.getXmrWalletService().getAddressEntry(offer.getId(), XmrAddressEntry.Context.OFFER_FUNDING).orElse(null); - Integer preferredSubaddressIndex = model.getOpenOffer().isSplitOutput() && fundingEntry != null ? fundingEntry.getSubaddressIndex() : null; - MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(makerFee, sendAmount, securityDeposit, returnAddress, exactOutputAmount, preferredSubaddressIndex); + Integer preferredSubaddressIndex = fundingEntry == null ? null : fundingEntry.getSubaddressIndex(); + MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(makerFee, sendAmount, securityDeposit, returnAddress, model.getOpenOffer().isSplitOutput(), preferredSubaddressIndex); // check for error in case creating reserve tx exceeded timeout // TODO: better way? diff --git a/core/src/main/java/haveno/core/support/dispute/DisputeManager.java b/core/src/main/java/haveno/core/support/dispute/DisputeManager.java index a8d0fb6e75..1cdc7b024a 100644 --- a/core/src/main/java/haveno/core/support/dispute/DisputeManager.java +++ b/core/src/main/java/haveno/core/support/dispute/DisputeManager.java @@ -61,7 +61,6 @@ import javafx.collections.FXCollections; import javafx.collections.ObservableList; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import monero.common.MoneroError; import monero.wallet.model.MoneroTxConfig; import monero.wallet.model.MoneroTxWallet; @@ -853,38 +852,24 @@ public abstract class DisputeManager> extends Sup if (winnerPayoutAmount.compareTo(BigInteger.ZERO) < 0) throw new RuntimeException("Winner payout cannot be negative"); if (loserPayoutAmount.compareTo(BigInteger.ZERO) < 0) throw new RuntimeException("Loser payout cannot be negative"); if (winnerPayoutAmount.add(loserPayoutAmount).compareTo(trade.getWallet().getUnlockedBalance()) > 0) { - throw new RuntimeException("The payout amounts are more than the wallet's unlocked balance"); + throw new RuntimeException("The payout amounts are more than the wallet's unlocked balance, unlocked balance=" + trade.getWallet().getUnlockedBalance() + " vs " + winnerPayoutAmount + " + " + loserPayoutAmount + " = " + (winnerPayoutAmount.add(loserPayoutAmount))); } // add any loss of precision to winner payout winnerPayoutAmount = winnerPayoutAmount.add(trade.getWallet().getUnlockedBalance().subtract(winnerPayoutAmount.add(loserPayoutAmount))); - // create transaction to get fee estimate - MoneroTxConfig txConfig = new MoneroTxConfig().setAccountIndex(0).setRelay(false); - if (winnerPayoutAmount.compareTo(BigInteger.ZERO) > 0) txConfig.addDestination(winnerPayoutAddress, winnerPayoutAmount.multiply(BigInteger.valueOf(9)).divide(BigInteger.valueOf(10))); // reduce payment amount to get fee of similar tx - if (loserPayoutAmount.compareTo(BigInteger.ZERO) > 0) txConfig.addDestination(loserPayoutAddress, loserPayoutAmount.multiply(BigInteger.valueOf(9)).divide(BigInteger.valueOf(10))); - MoneroTxWallet feeEstimateTx = trade.getWallet().createTx(txConfig); - - // create payout tx by increasing estimated fee until successful + // create dispute payout tx + MoneroTxConfig txConfig = new MoneroTxConfig().setAccountIndex(0); + if (winnerPayoutAmount.compareTo(BigInteger.ZERO) > 0) txConfig.addDestination(winnerPayoutAddress, winnerPayoutAmount); + if (loserPayoutAmount.compareTo(BigInteger.ZERO) > 0) txConfig.addDestination(loserPayoutAddress, loserPayoutAmount); + txConfig.setSubtractFeeFrom(loserPayoutAmount.equals(BigInteger.ZERO) ? 0 : txConfig.getDestinations().size() - 1); // winner only pays fee if loser gets 0 MoneroTxWallet payoutTx = null; - int numAttempts = 0; - while (payoutTx == null && numAttempts < 50) { - BigInteger feeEstimate = feeEstimateTx.getFee().add(feeEstimateTx.getFee().multiply(BigInteger.valueOf(numAttempts)).divide(BigInteger.valueOf(10))); // add 1/10th of fee until tx is successful - txConfig = new MoneroTxConfig().setAccountIndex(0).setRelay(false); - if (winnerPayoutAmount.compareTo(BigInteger.ZERO) > 0) txConfig.addDestination(winnerPayoutAddress, winnerPayoutAmount.subtract(loserPayoutAmount.equals(BigInteger.ZERO) ? feeEstimate : BigInteger.ZERO)); // winner only pays fee if loser gets 0 - if (loserPayoutAmount.compareTo(BigInteger.ZERO) > 0) { - if (loserPayoutAmount.compareTo(feeEstimate) < 0) throw new RuntimeException("Loser payout is too small to cover the mining fee"); - if (loserPayoutAmount.compareTo(feeEstimate) > 0) txConfig.addDestination(loserPayoutAddress, loserPayoutAmount.subtract(feeEstimate)); // loser pays fee - } - numAttempts++; - try { - payoutTx = trade.getWallet().createTx(txConfig); - } catch (MoneroError e) { - // exception expected // TODO: better way of estimating fee? - } + try { + payoutTx = trade.getWallet().createTx(txConfig); + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException("Loser payout is too small to cover the mining fee"); } - if (payoutTx == null) throw new RuntimeException("Failed to generate dispute payout tx after " + numAttempts + " attempts"); - log.info("Dispute payout transaction generated on attempt {}", numAttempts); // save updated multisig hex trade.getSelf().setUpdatedMultisigHex(trade.getWallet().exportMultisigHex()); diff --git a/core/src/main/java/haveno/core/support/dispute/arbitration/ArbitrationManager.java b/core/src/main/java/haveno/core/support/dispute/arbitration/ArbitrationManager.java index f3b90687ee..1525661f5a 100644 --- a/core/src/main/java/haveno/core/support/dispute/arbitration/ArbitrationManager.java +++ b/core/src/main/java/haveno/core/support/dispute/arbitration/ArbitrationManager.java @@ -388,9 +388,6 @@ public final class ArbitrationManager extends DisputeManager { } sendAckMessage(chatMessage, dispute.getAgentPubKeyRing(), true, null); - // set state after payout as we call swapTradeEntryToAvailableEntry + // set state after payout as we call swapAddressEntryToAvailable if (tradeManager.getOpenTrade(tradeId).isPresent()) { tradeManager.closeDisputedTrade(tradeId, Trade.DisputeState.REFUND_REQUEST_CLOSED); } else { diff --git a/core/src/main/java/haveno/core/trade/Trade.java b/core/src/main/java/haveno/core/trade/Trade.java index cc58c32365..51704e2379 100644 --- a/core/src/main/java/haveno/core/trade/Trade.java +++ b/core/src/main/java/haveno/core/trade/Trade.java @@ -630,7 +630,7 @@ public abstract class Trade implements Tradable, Model { if (isArbitrator() && !isCompleted()) processModel.getTradeManager().onTradeCompleted(this); // reset address entries - processModel.getXmrWalletService().resetAddressEntriesForPendingTrade(getId()); + processModel.getXmrWalletService().resetAddressEntriesForTrade(getId()); } // cleanup when payout unlocks @@ -922,32 +922,13 @@ public abstract class Trade implements Tradable, Model { BigInteger buyerPayoutAmount = buyerDepositAmount.add(tradeAmount); BigInteger sellerPayoutAmount = sellerDepositAmount.subtract(tradeAmount); - // create transaction to get fee estimate - MoneroTxWallet feeEstimateTx = multisigWallet.createTx(new MoneroTxConfig() + // create payout tx + MoneroTxWallet payoutTx = multisigWallet.createTx(new MoneroTxConfig() .setAccountIndex(0) - .addDestination(buyerPayoutAddress, buyerPayoutAmount.multiply(BigInteger.valueOf(9)).divide(BigInteger.valueOf(10))) // reduce payment amount to compute fee of similar tx - .addDestination(sellerPayoutAddress, sellerPayoutAmount.multiply(BigInteger.valueOf(9)).divide(BigInteger.valueOf(10))) - .setRelay(false) - ); - - // attempt to create payout tx by increasing estimated fee until successful - MoneroTxWallet payoutTx = null; - int numAttempts = 0; - while (payoutTx == null && numAttempts < 50) { - BigInteger feeEstimate = feeEstimateTx.getFee().add(feeEstimateTx.getFee().multiply(BigInteger.valueOf(numAttempts)).divide(BigInteger.valueOf(10))); // add 1/10 of fee until tx is successful - try { - numAttempts++; - payoutTx = multisigWallet.createTx(new MoneroTxConfig() - .setAccountIndex(0) - .addDestination(buyerPayoutAddress, buyerPayoutAmount.subtract(feeEstimate.divide(BigInteger.valueOf(2)))) // split fee subtracted from each payout amount - .addDestination(sellerPayoutAddress, sellerPayoutAmount.subtract(feeEstimate.divide(BigInteger.valueOf(2)))) - .setRelay(false)); - } catch (MoneroError e) { - // exception expected - } - } - if (payoutTx == null) throw new RuntimeException("Failed to generate payout tx after " + numAttempts + " attempts"); - log.info("Payout transaction generated on attempt {}", numAttempts); + .addDestination(buyerPayoutAddress, buyerPayoutAmount) + .addDestination(sellerPayoutAddress, sellerPayoutAmount) + .setSubtractFeeFrom(0, 1) // split tx fee + .setRelay(false)); // save updated multisig hex getSelf().setUpdatedMultisigHex(multisigWallet.exportMultisigHex()); diff --git a/core/src/main/java/haveno/core/trade/TradeManager.java b/core/src/main/java/haveno/core/trade/TradeManager.java index bdfeb688ee..cc021219ae 100644 --- a/core/src/main/java/haveno/core/trade/TradeManager.java +++ b/core/src/main/java/haveno/core/trade/TradeManager.java @@ -356,6 +356,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi reservedKeyImages.addAll(trade.getSelf().getReserveTxKeyImages()); } for (OpenOffer openOffer : openOfferManager.getObservableList()) { + if (openOffer.getOffer().getOfferPayload().getReserveTxKeyImages() == null) continue; reservedKeyImages.addAll(openOffer.getOffer().getOfferPayload().getReserveTxKeyImages()); } @@ -473,7 +474,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi .filter(addressEntry -> addressEntry.getOfferId() != null) .forEach(addressEntry -> { log.warn("Swapping pending {} entries at startup. offerId={}", addressEntry.getContext(), addressEntry.getOfferId()); - xmrWalletService.swapTradeEntryToAvailableEntry(addressEntry.getOfferId(), addressEntry.getContext()); + xmrWalletService.swapAddressEntryToAvailable(addressEntry.getOfferId(), addressEntry.getContext()); }); onTradesInitiailizedAndAppFullyInitialized(); @@ -946,7 +947,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi removeTrade(trade); // TODO The address entry should have been removed already. Check and if its the case remove that. - xmrWalletService.resetAddressEntriesForPendingTrade(trade.getId()); + xmrWalletService.resetAddressEntriesForTrade(trade.getId()); requestPersistence(); } @@ -961,7 +962,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi Trade trade = tradeOptional.get(); trade.setDisputeState(disputeState); onTradeCompleted(trade); - xmrWalletService.swapTradeEntryToAvailableEntry(trade.getId(), XmrAddressEntry.Context.TRADE_PAYOUT); + xmrWalletService.resetAddressEntriesForTrade(trade.getId()); requestPersistence(); } } diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java b/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java index 860a7f0af1..56c33d95fb 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java @@ -22,6 +22,7 @@ import common.utils.JsonUtils; import haveno.common.app.Version; import haveno.common.crypto.PubKeyRing; import haveno.common.taskrunner.TaskRunner; +import haveno.common.util.Tuple2; import haveno.core.offer.Offer; import haveno.core.trade.HavenoUtils; import haveno.core.trade.Trade; @@ -33,6 +34,7 @@ import haveno.network.p2p.SendDirectMessageListener; import lombok.extern.slf4j.Slf4j; import monero.daemon.MoneroDaemon; import monero.daemon.model.MoneroSubmitTxResult; +import monero.daemon.model.MoneroTx; import java.math.BigInteger; import java.util.Arrays; @@ -83,8 +85,9 @@ public class ArbitratorProcessDepositRequest extends TradeTask { String depositAddress = processModel.getMultisigAddress(); // verify deposit tx + Tuple2 txResult; try { - trade.getXmrWalletService().verifyTradeTx( + txResult = trade.getXmrWalletService().verifyTradeTx( offer.getId(), tradeFee, sendAmount, @@ -100,6 +103,7 @@ public class ArbitratorProcessDepositRequest extends TradeTask { } // set deposit info + trader.setSecurityDeposit(txResult.second); trader.setDepositTxHex(request.getDepositTxHex()); trader.setDepositTxKey(request.getDepositTxKey()); if (request.getPaymentAccountKey() != null) trader.setPaymentAccountKey(request.getPaymentAccountKey()); diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java b/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java index 0b04ebc58e..23c66a1587 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java @@ -31,7 +31,6 @@ import lombok.extern.slf4j.Slf4j; import monero.daemon.model.MoneroOutput; import monero.wallet.model.MoneroTxWallet; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -77,15 +76,12 @@ public class MaybeSendSignContractRequest extends TradeTask { // create deposit tx and freeze inputs Integer subaddressIndex = null; - BigInteger exactOutputAmount = null; + boolean isSplitOutputOffer = false; if (trade instanceof MakerTrade) { - boolean isSplitOutputOffer = processModel.getOpenOfferManager().getOpenOfferById(trade.getId()).get().isSplitOutput(); - if (isSplitOutputOffer) { - exactOutputAmount = trade.getOffer().getReserveAmount(); - subaddressIndex = model.getXmrWalletService().getAddressEntry(trade.getId(), XmrAddressEntry.Context.OFFER_FUNDING).get().getSubaddressIndex(); - } + isSplitOutputOffer = processModel.getOpenOfferManager().getOpenOfferById(trade.getId()).get().isSplitOutput(); + if (isSplitOutputOffer) subaddressIndex = model.getXmrWalletService().getAddressEntry(trade.getId(), XmrAddressEntry.Context.OFFER_FUNDING).get().getSubaddressIndex(); } - MoneroTxWallet depositTx = trade.getXmrWalletService().createDepositTx(trade, exactOutputAmount, subaddressIndex); + MoneroTxWallet depositTx = trade.getXmrWalletService().createDepositTx(trade, isSplitOutputOffer, subaddressIndex); // collect reserved key images List reservedKeyImages = new ArrayList(); diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/RemoveOffer.java b/core/src/main/java/haveno/core/trade/protocol/tasks/RemoveOffer.java index c5b8a3f5b2..18dcf0c8dc 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/RemoveOffer.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/RemoveOffer.java @@ -38,7 +38,7 @@ public class RemoveOffer extends TradeTask { if (trade instanceof MakerTrade) { processModel.getOpenOfferManager().closeOpenOffer(checkNotNull(trade.getOffer())); } else { - trade.getXmrWalletService().resetAddressEntriesForOpenOffer(trade.getId()); + trade.getXmrWalletService().resetOfferFundingForOpenOffer(trade.getId()); } complete(); diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/SellerPublishDepositTx.java b/core/src/main/java/haveno/core/trade/protocol/tasks/SellerPublishDepositTx.java index b6ebcc131a..36bd2a19d5 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/SellerPublishDepositTx.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/SellerPublishDepositTx.java @@ -44,7 +44,7 @@ public class SellerPublishDepositTx extends TradeTask { // // trade.setState(Trade.State.SELLER_PUBLISHED_DEPOSIT_TX); // -// processModel.getBtcWalletService().swapTradeEntryToAvailableEntry(processModel.getOffer().getId(), +// processModel.getBtcWalletService().swapAddressEntryToAvailable(processModel.getOffer().getId(), // AddressEntry.Context.RESERVED_FOR_TRADE); // // processModel.getTradeManager().requestPersistence(); diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/TakerReserveTradeFunds.java b/core/src/main/java/haveno/core/trade/protocol/tasks/TakerReserveTradeFunds.java index 69f6bb5fd4..952487a24a 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/TakerReserveTradeFunds.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/TakerReserveTradeFunds.java @@ -44,7 +44,7 @@ public class TakerReserveTradeFunds extends TradeTask { BigInteger sendAmount = trade.getOffer().getDirection() == OfferDirection.BUY ? trade.getOffer().getAmount() : BigInteger.valueOf(0); BigInteger securityDeposit = trade.getOffer().getDirection() == OfferDirection.BUY ? trade.getOffer().getSellerSecurityDeposit() : trade.getOffer().getBuyerSecurityDeposit(); String returnAddress = model.getXmrWalletService().getOrCreateAddressEntry(trade.getOffer().getId(), XmrAddressEntry.Context.TRADE_PAYOUT).getAddressString(); - MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(takerFee, sendAmount, securityDeposit, returnAddress, null, null); + MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(takerFee, sendAmount, securityDeposit, returnAddress, false, null); // collect reserved key images List reservedKeyImages = new ArrayList(); diff --git a/core/src/main/java/haveno/core/util/VolumeUtil.java b/core/src/main/java/haveno/core/util/VolumeUtil.java index d607f1711b..38b759994e 100644 --- a/core/src/main/java/haveno/core/util/VolumeUtil.java +++ b/core/src/main/java/haveno/core/util/VolumeUtil.java @@ -131,7 +131,7 @@ public class VolumeUtil { } public static String formatVolume(Volume volume) { - return formatVolume(volume, getMonetaryFormat(volume.getCurrencyCode()), false); + return volume == null ? "" : formatVolume(volume, getMonetaryFormat(volume.getCurrencyCode()), false); } private static String formatVolume(Volume volume, MonetaryFormat volumeFormat, boolean appendCurrencyCode) { diff --git a/core/src/main/java/haveno/core/xmr/wallet/BtcWalletService.java b/core/src/main/java/haveno/core/xmr/wallet/BtcWalletService.java index b7a884dae4..6a088f3b96 100644 --- a/core/src/main/java/haveno/core/xmr/wallet/BtcWalletService.java +++ b/core/src/main/java/haveno/core/xmr/wallet/BtcWalletService.java @@ -289,9 +289,9 @@ public class BtcWalletService extends WalletService { return addressEntryList.getAddressEntriesAsListImmutable(); } - public void swapTradeEntryToAvailableEntry(String offerId, AddressEntry.Context context) { + public void swapAddressEntryToAvailable(String offerId, AddressEntry.Context context) { if (context == AddressEntry.Context.MULTI_SIG) { - log.error("swapTradeEntryToAvailableEntry called with MULTI_SIG context. " + + log.error("swapAddressEntryToAvailable called with MULTI_SIG context. " + "This in not permitted as we must not reuse those address entries and there " + "are no redeemable funds on that addresses. Only the keys are used for creating " + "the Multisig address. offerId={}, context={}", offerId, context); @@ -327,8 +327,8 @@ public class BtcWalletService extends WalletService { public void resetAddressEntriesForOpenOffer(String offerId) { log.info("resetAddressEntriesForOpenOffer offerId={}", offerId); - swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.OFFER_FUNDING); - swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.RESERVED_FOR_TRADE); + swapAddressEntryToAvailable(offerId, AddressEntry.Context.OFFER_FUNDING); + swapAddressEntryToAvailable(offerId, AddressEntry.Context.RESERVED_FOR_TRADE); } public void resetAddressEntriesForPendingTrade(String offerId) { @@ -342,7 +342,7 @@ public class BtcWalletService extends WalletService { // 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); + swapAddressEntryToAvailable(offerId, AddressEntry.Context.TRADE_PAYOUT); } public void swapAnyTradeEntryContextToAvailableEntry(String offerId) { diff --git a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java index 1cfd0f4eba..e79d166cce 100644 --- a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java +++ b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java @@ -57,17 +57,18 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import java.io.File; -import java.math.BigDecimal; import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; 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 java.util.TreeSet; import java.util.concurrent.Callable; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.ExecutorService; @@ -93,8 +94,6 @@ public class XmrWalletService { private static final String MONERO_WALLET_RPC_DEFAULT_PASSWORD = "password"; // only used if account password is null private static final String MONERO_WALLET_NAME = "haveno_XMR"; public static final double MINER_FEE_TOLERANCE = 0.25; // miner fee must be within percent of estimated fee - private static final double SECURITY_DEPOSIT_TOLERANCE = Config.baseCurrencyNetwork() == BaseCurrencyNetwork.XMR_LOCAL ? 0.25 : 0.05; // security deposit can absorb miner fee up to percent - private static final double DUST_TOLERANCE = 0.01; // max dust as percent of mining fee private static final int NUM_MAX_BACKUP_WALLETS = 10; private static final int MONERO_LOG_LEVEL = 0; private static final boolean PRINT_STACK_TRACE = false; @@ -322,50 +321,55 @@ public class XmrWalletService { } } + private List getSubaddressesWithExactInput(BigInteger amount) { + + // fetch unspent, unfrozen, unlocked outputs + List exactOutputs = wallet.getOutputs(new MoneroOutputQuery() + .setAmount(amount) + .setIsSpent(false) + .setIsFrozen(false) + .setTxQuery(new MoneroTxQuery().setIsLocked(false))); + + // collect subaddresses indices as sorted set + TreeSet subaddressIndices = new TreeSet(); + for (MoneroOutputWallet output : exactOutputs) subaddressIndices.add(output.getSubaddressIndex()); + return new ArrayList(subaddressIndices); + } + /** * Create the reserve tx and freeze its inputs. The full amount is returned - * to the sender's payout address less the trade fee. + * to the sender's payout address less the security deposit and mining fee. * * @param tradeFee trade fee * @param sendAmount amount to give peer * @param securityDeposit security deposit amount * @param returnAddress return address for reserved funds - * @param exactOutputAmount exact output amount to spend (optional) - * @param subaddressIndex preferred source subaddress to spend from (optional) + * @param reserveExactAmount specifies to reserve the exact input amount + * @param preferredSubaddressIndex preferred source subaddress to spend from (optional) * @return a transaction to reserve a trade */ - public MoneroTxWallet createReserveTx(BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String returnAddress, BigInteger exactOutputAmount, Integer subaddressIndex) { - log.info("Creating reserve tx with return address={}", returnAddress); + public MoneroTxWallet createReserveTx(BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String returnAddress, boolean reserveExactAmount, Integer preferredSubaddressIndex) { + log.info("Creating reserve tx with preferred subaddress index={}, return address={}", preferredSubaddressIndex, returnAddress); long time = System.currentTimeMillis(); - try { - MoneroTxWallet reserveTx = createTradeTx(tradeFee, sendAmount, securityDeposit, returnAddress, true, exactOutputAmount, subaddressIndex); - log.info("Done creating reserve tx in {} ms", System.currentTimeMillis() - time); - return reserveTx; - } catch (Exception e) { - if (exactOutputAmount != null) return spendOutputManually(true, tradeFee, sendAmount, securityDeposit, returnAddress, exactOutputAmount); - - // retry creating reserve tx using funds outside subaddress - if (subaddressIndex != null) return createReserveTx(tradeFee, sendAmount, securityDeposit, returnAddress, exactOutputAmount, null); - else throw e; - } + MoneroTxWallet reserveTx = createTradeTx(tradeFee, sendAmount, securityDeposit, returnAddress, true, reserveExactAmount, preferredSubaddressIndex); + log.info("Done creating reserve tx in {} ms", System.currentTimeMillis() - time); + return reserveTx; } - /** + /**s * Create the multisig deposit tx and freeze its inputs. * * @param trade the trade to create a deposit tx from - * @param exactOutputAmount exact output amount to spend (optional) - * @param subaddressIndex preferred source subaddress to spend from (optional) + * @param reserveExactAmount specifies to reserve the exact input amount + * @param preferredSubaddressIndex preferred source subaddress to spend from (optional) * @return MoneroTxWallet the multisig deposit tx */ - public MoneroTxWallet createDepositTx(Trade trade, BigInteger exactOutputAmount, Integer subaddressIndex) { + public MoneroTxWallet createDepositTx(Trade trade, boolean reserveExactAmount, Integer preferredSubaddressIndex) { Offer offer = trade.getProcessModel().getOffer(); String multisigAddress = trade.getProcessModel().getMultisigAddress(); BigInteger tradeFee = trade instanceof MakerTrade ? trade.getOffer().getMakerFee() : trade.getTakerFee(); BigInteger sendAmount = trade instanceof BuyerTrade ? BigInteger.valueOf(0) : offer.getAmount(); BigInteger securityDeposit = trade instanceof BuyerTrade ? offer.getBuyerSecurityDeposit() : offer.getSellerSecurityDeposit(); - - // thaw reserved outputs then create deposit tx MoneroWallet wallet = getWallet(); synchronized (wallet) { @@ -374,98 +378,76 @@ public class XmrWalletService { thawOutputs(trade.getSelf().getReserveTxKeyImages()); } - log.info("Creating deposit tx for trade {} {} with multisig address={}", trade.getClass().getSimpleName(), trade.getId(), multisigAddress); + // create deposit tx long time = System.currentTimeMillis(); - try { - MoneroTxWallet tradeTx = createTradeTx(tradeFee, sendAmount, securityDeposit, multisigAddress, false, exactOutputAmount, subaddressIndex); - log.info("Done creating deposit tx for trade {} {} in {} ms", trade.getClass().getSimpleName(), trade.getId(), System.currentTimeMillis() - time); - return tradeTx; - } catch (Exception e) { - if (exactOutputAmount != null) return spendOutputManually(false, tradeFee, sendAmount, securityDeposit, multisigAddress, exactOutputAmount); - - // retry creating deposit tx using funds outside subaddress - if (subaddressIndex != null) return createDepositTx(trade, exactOutputAmount, null); - else throw e; - } + log.info("Creating deposit tx with multisig address={}", multisigAddress); + MoneroTxWallet depositTx = createTradeTx(tradeFee, sendAmount, securityDeposit, multisigAddress, false, reserveExactAmount, preferredSubaddressIndex); + log.info("Done creating deposit tx for trade {} {} in {} ms", trade.getClass().getSimpleName(), trade.getId(), System.currentTimeMillis() - time); + return depositTx; } } - // retry with exact outputs in other subaddresses - // TODO: this is a hack because wallet2 sometimes prefers to spend multiple inputs intead of exact output; replace with fund by destination address when available - private MoneroTxWallet spendOutputManually(boolean isReserveTx, BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String returnAddress, BigInteger exactOutputAmount) { - log.warn("Manually selecting subaddress to spend output from"); - List exactOutputs = wallet.getOutputs(new MoneroOutputQuery() - .setAmount(exactOutputAmount) - .setIsSpent(false) - .setIsFrozen(false)); - Set subaddressIndices = new HashSet(); - for (MoneroOutputWallet output : exactOutputs) { - if (!output.getTx().isLocked()) subaddressIndices.add(output.getSubaddressIndex()); - } - Exception err = null; - for (Integer idx : subaddressIndices) { - try { - long startTime = System.currentTimeMillis(); - MoneroTxWallet reserveTx = createTradeTx(tradeFee, sendAmount, securityDeposit, returnAddress, isReserveTx, exactOutputAmount, idx); - log.info("Done creating output tx in {} ms", System.currentTimeMillis() - startTime); - return reserveTx; - } catch (Exception e2) { - err = e2; - } - } - if (err != null) throw new RuntimeException(err); - throw new RuntimeException("No output available with amount " + exactOutputAmount); - } - - private MoneroTxWallet createTradeTx(BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String address, boolean isReserveTx, BigInteger exactOutputAmount, Integer subaddressIndex) { + private MoneroTxWallet createTradeTx(BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String address, boolean isReserveTx, boolean reserveExactAmount, Integer preferredSubaddressIndex) { MoneroWallet wallet = getWallet(); synchronized (wallet) { - // binary search to maximize security deposit and minimize potential dust - // TODO: binary search is hacky and slow over TOR connections, replace with destination paying tx fee - MoneroTxWallet tradeTx = null; - double appliedTolerance = 0.0; // percent of tolerance to apply, thereby decreasing security deposit - double searchDiff = 1.0; // difference for next binary search - int maxSearches = 5; - for (int i = 0; i < maxSearches; i++) { - try { - BigInteger appliedSecurityDeposit = new BigDecimal(securityDeposit).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE * appliedTolerance)).toBigInteger(); - BigInteger amount = sendAmount.add(isReserveTx ? tradeFee : appliedSecurityDeposit); - MoneroTxWallet testTx = wallet.createTx(new MoneroTxConfig() - .setAccountIndex(0) - .setSubaddressIndices(subaddressIndex == null ? null : Arrays.asList(subaddressIndex)) // TODO monero-java: MoneroTxConfig.setSubadddressIndex(int) causes NPE with null subaddress, could setSubaddressIndices(null) as convenience - .addDestination(HavenoUtils.getTradeFeeAddress(), isReserveTx ? appliedSecurityDeposit : tradeFee) // reserve tx charges security deposit if published - .addDestination(address, amount)); - - // assert exact input if expected - if (exactOutputAmount == null) { - tradeTx = testTx; - } else { - BigInteger inputSum = BigInteger.valueOf(0); - for (MoneroOutputWallet txInput : testTx.getInputsWallet()) { - MoneroOutputWallet input = wallet.getOutputs(new MoneroOutputQuery().setKeyImage(txInput.getKeyImage())).get(0); - inputSum = inputSum.add(input.getAmount()); - } - if (inputSum.compareTo(exactOutputAmount) > 0) throw new RuntimeException("Spending too much since input sum is greater than output amount"); // continues binary search with less security deposit - else if (inputSum.equals(exactOutputAmount) && testTx.getInputs().size() == 1) tradeTx = testTx; - } - appliedTolerance -= searchDiff; // apply less tolerance to increase security deposit - if (appliedTolerance < 0.0) break; // can send full security deposit - } catch (Exception e) { - appliedTolerance += searchDiff; // apply more tolerance to decrease security deposit - if (appliedTolerance > 1.0) { - if (tradeTx == null) throw e; - break; - } + // create a list of subaddresses to attempt spending from in preferred order + List subaddressIndices = new ArrayList(); + if (reserveExactAmount) { + BigInteger exactInputAmount = tradeFee.add(sendAmount).add(securityDeposit); + List subaddressIndicesWithExactInput = getSubaddressesWithExactInput(exactInputAmount); + if (preferredSubaddressIndex != null) subaddressIndicesWithExactInput.remove(preferredSubaddressIndex); + Collections.sort(subaddressIndicesWithExactInput); + Collections.reverse(subaddressIndicesWithExactInput); + subaddressIndices.addAll(subaddressIndicesWithExactInput); + } + if (preferredSubaddressIndex != null) { + if (wallet.getBalance(0, preferredSubaddressIndex).compareTo(BigInteger.valueOf(0)) > 0) { + subaddressIndices.add(0, preferredSubaddressIndex); // try preferred subaddress first if funded + } else if (reserveExactAmount) { + subaddressIndices.add(preferredSubaddressIndex); // otherwise only try preferred subaddress if using exact output } - searchDiff /= 2; + } + + // first try preferred subaddressess + for (int i = 0; i < subaddressIndices.size(); i++) { + try { + return createTradeTxFromSubaddress(tradeFee, sendAmount, securityDeposit, address, isReserveTx, reserveExactAmount, subaddressIndices.get(i)); + } catch (Exception e) { + if (i == subaddressIndices.size() - 1 && reserveExactAmount) throw e; // throw if no subaddress with exact output + } + } + + // try any subaddress + return createTradeTxFromSubaddress(tradeFee, sendAmount, securityDeposit, address, isReserveTx, reserveExactAmount, null); + } + } + + private MoneroTxWallet createTradeTxFromSubaddress(BigInteger tradeFee, BigInteger sendAmount, BigInteger securityDeposit, String address, boolean isReserveTx, boolean reserveExactAmount, Integer subaddressIndex) { + + // create tx + MoneroTxWallet tradeTx = wallet.createTx(new MoneroTxConfig() + .setAccountIndex(0) + .setSubaddressIndices(subaddressIndex) + .addDestination(HavenoUtils.getTradeFeeAddress(), isReserveTx ? securityDeposit : tradeFee) // reserve tx charges security deposit if published + .addDestination(address, sendAmount.add(isReserveTx ? tradeFee : securityDeposit)) + .setSubtractFeeFrom(isReserveTx ? 0 : 1)); // pay fee from same destination as security deposit + + // check if tx uses exact input, since wallet2 can prefer to spend 2 outputs + if (reserveExactAmount) { + BigInteger exactInputAmount = tradeFee.add(sendAmount).add(securityDeposit); + BigInteger inputSum = BigInteger.valueOf(0); + for (MoneroOutputWallet txInput : tradeTx.getInputsWallet()) { + MoneroOutputWallet input = wallet.getOutputs(new MoneroOutputQuery().setKeyImage(txInput.getKeyImage())).get(0); + inputSum = inputSum.add(input.getAmount()); + } + if (inputSum.compareTo(exactInputAmount) > 0) throw new RuntimeException("Cannot create transaction with exact input amount"); } // freeze inputs for (MoneroOutput input : tradeTx.getInputs()) wallet.freezeOutput(input.getKeyImage().getHex()); saveMainWallet(); return tradeTx; - } } /** @@ -533,19 +515,20 @@ public class XmrWalletService { BigInteger actualSendAmount = transferCheck.getReceivedAmount().subtract(isReserveTx ? actualTradeFee : actualSecurityDeposit); // verify trade fee - if (!tradeFee.equals(actualTradeFee)) { - throw new RuntimeException("Trade fee is incorrect amount, expected=" + tradeFee + ", actual=" + actualTradeFee + ", transfer address check=" + JsonUtils.serialize(transferCheck) + ", trade fee address check=" + JsonUtils.serialize(tradeFeeCheck)); + if (actualTradeFee.compareTo(tradeFee) < 0) { + throw new RuntimeException("Insufficient trade fee, expected=" + tradeFee + ", actual=" + actualTradeFee + ", transfer address check=" + JsonUtils.serialize(transferCheck) + ", trade fee address check=" + JsonUtils.serialize(tradeFeeCheck)); } - // verify sufficient security deposit - BigInteger minSecurityDeposit = new BigDecimal(securityDeposit).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE)).toBigInteger(); - if (actualSecurityDeposit.compareTo(minSecurityDeposit) < 0) throw new RuntimeException("Security deposit amount is not enough, needed " + minSecurityDeposit + " but was " + actualSecurityDeposit); + // verify send amount + if (!actualSendAmount.equals(sendAmount)) { + throw new RuntimeException("Unexpected send amount, expected " + sendAmount + " but was " + actualSendAmount); + } - // verify deposit amount + miner fee within dust tolerance - //BigInteger minDepositAndFee = sendAmount.add(securityDeposit).subtract(new BigDecimal(tx.getFee()).multiply(new BigDecimal(1.0 - DUST_TOLERANCE)).toBigInteger()); // TODO: improve when destination pays fee - BigInteger minDeposit = sendAmount.add(minSecurityDeposit); - BigInteger actualDeposit = actualSendAmount.add(actualSecurityDeposit); - if (actualDeposit.compareTo(minDeposit) < 0) throw new RuntimeException("Deposit amount + fee is not enough, needed " + minDeposit + " but was " + actualDeposit); + // verify security deposit + BigInteger expectedSecurityDeposit = securityDeposit.subtract(tx.getFee()); // fee is paid from security deposit + if (!actualSecurityDeposit.equals(expectedSecurityDeposit)) { + throw new RuntimeException("Unexpected security deposit amount, expected " + expectedSecurityDeposit + " but was " + actualSecurityDeposit); + } } catch (Exception e) { log.warn("Error verifying trade tx with offer id=" + offerId + (tx == null ? "" : ", tx=" + tx) + ": " + e.getMessage()); throw e; @@ -926,8 +909,8 @@ public class XmrWalletService { // try to use available and not yet used entries try { List incomingTxs = getTxsWithIncomingOutputs(); // prefetch all incoming txs to avoid query per subaddress - Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream().filter(e -> XmrAddressEntry.Context.AVAILABLE == e.getContext()).filter(e -> isSubaddressUnused(e.getSubaddressIndex(), incomingTxs)).findAny(); - if (emptyAvailableAddressEntry.isPresent()) return xmrAddressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); + List unusedAddressEntries = getUnusedAddressEntries(incomingTxs); + if (!unusedAddressEntries.isEmpty()) return xmrAddressEntryList.swapAvailableToAddressEntryWithOfferId(unusedAddressEntries.get(0), context, offerId); } catch (Exception e) { log.warn("Error getting new address entry based on incoming transactions"); e.printStackTrace(); @@ -983,7 +966,7 @@ public class XmrWalletService { return entries.isEmpty() ? Optional.empty() : Optional.of(entries.get(0)); } - public synchronized void swapTradeEntryToAvailableEntry(String offerId, XmrAddressEntry.Context context) { + public synchronized void swapAddressEntryToAvailable(String offerId, XmrAddressEntry.Context context) { Optional addressEntryOptional = getAddressEntryListAsImmutableList().stream().filter(e -> offerId.equals(e.getOfferId())).filter(e -> context == e.getContext()).findAny(); addressEntryOptional.ifPresent(e -> { log.info("swap addressEntry with address {} and offerId {} from context {} to available", e.getAddressString(), e.getOfferId(), context); @@ -994,22 +977,17 @@ public class XmrWalletService { public synchronized void resetAddressEntriesForOpenOffer(String offerId) { log.info("resetAddressEntriesForOpenOffer offerId={}", offerId); - swapTradeEntryToAvailableEntry(offerId, XmrAddressEntry.Context.OFFER_FUNDING); + swapAddressEntryToAvailable(offerId, XmrAddressEntry.Context.OFFER_FUNDING); + swapAddressEntryToAvailable(offerId, XmrAddressEntry.Context.TRADE_PAYOUT); } - public synchronized void resetAddressEntriesForPendingTrade(String offerId) { - // 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 Haveno 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, XmrAddressEntry.Context.TRADE_PAYOUT); + public synchronized void resetOfferFundingForOpenOffer(String offerId) { + log.info("resetOfferFundingForOpenOffer offerId={}", offerId); + swapAddressEntryToAvailable(offerId, XmrAddressEntry.Context.OFFER_FUNDING); + } + + public synchronized void resetAddressEntriesForTrade(String offerId) { + swapAddressEntryToAvailable(offerId, XmrAddressEntry.Context.TRADE_PAYOUT); } private Optional findAddressEntry(String address, XmrAddressEntry.Context context) { @@ -1063,32 +1041,36 @@ public class XmrWalletService { public List getUnusedAddressEntries(List cachedTxs) { return getAvailableAddressEntries().stream() - .filter(e -> isSubaddressUnused(e.getSubaddressIndex(), cachedTxs)) + .filter(e -> e.getContext() == XmrAddressEntry.Context.AVAILABLE && !subaddressHasIncomingTransfers(e.getSubaddressIndex(), cachedTxs)) .collect(Collectors.toList()); } - public boolean isSubaddressUnused(int subaddressIndex) { - return isSubaddressUnused(subaddressIndex, null); + public boolean subaddressHasIncomingTransfers(int subaddressIndex) { + return subaddressHasIncomingTransfers(subaddressIndex, null); } - private boolean isSubaddressUnused(int subaddressIndex, List incomingTxs) { - return getNumOutputsForSubaddress(subaddressIndex, incomingTxs) == 0; - } - - public int getNumOutputsForSubaddress(int subaddressIndex) { - return getNumOutputsForSubaddress(subaddressIndex, null); + private boolean subaddressHasIncomingTransfers(int subaddressIndex, List incomingTxs) { + return getNumOutputsForSubaddress(subaddressIndex, incomingTxs) > 0; } public int getNumOutputsForSubaddress(int subaddressIndex, List incomingTxs) { - if (incomingTxs == null) incomingTxs = getTxsWithIncomingOutputs(subaddressIndex); + incomingTxs = getTxsWithIncomingOutputs(subaddressIndex, incomingTxs); int numUnspentOutputs = 0; for (MoneroTxWallet tx : incomingTxs) { //if (tx.getTransfers(new MoneroTransferQuery().setSubaddressIndex(subaddressIndex)).isEmpty()) continue; // TODO monero-project: transfers are occluded by transfers from/to same account, so this will return unused when used numUnspentOutputs += tx.isConfirmed() ? tx.getOutputsWallet(new MoneroOutputQuery().setAccountIndex(0).setSubaddressIndex(subaddressIndex)).size() : 1; // TODO: monero-project does not provide outputs for unconfirmed txs } + boolean positiveBalance = wallet.getBalance(0, subaddressIndex).compareTo(BigInteger.valueOf(0)) > 0; + if (positiveBalance && numUnspentOutputs == 0) return 1; // outputs do not appear until confirmed and internal transfers are occluded, so report 1 if positive balance return numUnspentOutputs; } + public int getNumTxsWithIncomingOutputs(int subaddressIndex, List txs) { + List txsWithIncomingOutputs = getTxsWithIncomingOutputs(subaddressIndex, txs); + if (txsWithIncomingOutputs.isEmpty() && subaddressHasIncomingTransfers(subaddressIndex, txsWithIncomingOutputs)) return 1; // outputs do not appear until confirmed and internal transfers are occluded, so report 1 if positive balance + return txsWithIncomingOutputs.size(); + } + public List getTxsWithIncomingOutputs() { return getTxsWithIncomingOutputs(null); } diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index d3cfcbef2e..a91db43769 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -1016,6 +1016,9 @@ funds.tab.transactions=Transactions funds.deposit.unused=Unused funds.deposit.usedInTx=Used in {0} transaction(s) +funds.deposit.baseAddress=Base address +funds.deposit.offerFunding=Reserved for offer funding +funds.deposit.tradePayout=Reserved for trade payout funds.deposit.fundHavenoWallet=Fund Haveno wallet funds.deposit.noAddresses=No deposit addresses have been generated yet funds.deposit.fundWallet=Fund your wallet diff --git a/desktop/src/main/java/haveno/desktop/main/MainViewModel.java b/desktop/src/main/java/haveno/desktop/main/MainViewModel.java index 0f35849adf..86ed1a25dd 100644 --- a/desktop/src/main/java/haveno/desktop/main/MainViewModel.java +++ b/desktop/src/main/java/haveno/desktop/main/MainViewModel.java @@ -588,7 +588,7 @@ public class MainViewModel implements ViewModel, HavenoSetup.HavenoSetupListener .width(1000) .actionButtonText(Res.get("shared.removeOffer")) .onAction(() -> { - openOfferManager.removeOpenOffer(openOffer, () -> { + openOfferManager.cancelOpenOffer(openOffer, () -> { log.info("Invalid open offer with ID {} was successfully removed.", openOffer.getId()); }, log::error); diff --git a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositListItem.java b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositListItem.java index ec05805d4a..d0f302c2b8 100644 --- a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositListItem.java +++ b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositListItem.java @@ -95,8 +95,23 @@ class DepositListItem { } private void updateUsage(int subaddressIndex, List cachedTxs) { - numTxsWithOutputs = xmrWalletService.getTxsWithIncomingOutputs(addressEntry.getSubaddressIndex(), cachedTxs).size(); - usage = subaddressIndex == 0 ? "Base address" : numTxsWithOutputs == 0 ? Res.get("funds.deposit.unused") : Res.get("funds.deposit.usedInTx", numTxsWithOutputs); + numTxsWithOutputs = xmrWalletService.getNumTxsWithIncomingOutputs(addressEntry.getSubaddressIndex(), cachedTxs); + switch (addressEntry.getContext()) { + case BASE_ADDRESS: + usage = Res.get("funds.deposit.baseAddress"); + break; + case AVAILABLE: + usage = numTxsWithOutputs == 0 ? Res.get("funds.deposit.unused") : Res.get("funds.deposit.usedInTx", numTxsWithOutputs); + break; + case OFFER_FUNDING: + usage = Res.get("funds.deposit.offerFunding"); + break; + case TRADE_PAYOUT: + usage = Res.get("funds.deposit.tradePayout"); + break; + default: + usage = addressEntry.getContext().toString(); + } } public void cleanup() { diff --git a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java index 7bc3f900bd..07dc378394 100644 --- a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java +++ b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java @@ -156,7 +156,7 @@ public class DepositView extends ActivatableView { addressColumn.setComparator(Comparator.comparing(DepositListItem::getAddressString)); balanceColumn.setComparator(Comparator.comparing(DepositListItem::getBalanceAsBI)); confirmationsColumn.setComparator(Comparator.comparingLong(o -> o.getNumConfirmationsSinceFirstUsed(txsWithIncomingOutputs))); - usageColumn.setComparator(Comparator.comparingInt(DepositListItem::getNumTxsWithOutputs)); + usageColumn.setComparator(Comparator.comparing(DepositListItem::getUsage)); tableView.getSortOrder().add(usageColumn); tableView.setItems(sortedList); @@ -202,8 +202,8 @@ public class DepositView extends ActivatableView { generateNewAddressButton = buttonCheckBoxHBox.first; generateNewAddressButton.setOnAction(event -> { - boolean hasUnUsedAddress = observableList.stream().anyMatch(e -> e.getSubaddressIndex() != 0 && xmrWalletService.getTxsWithIncomingOutputs(e.getSubaddressIndex()).isEmpty()); - if (hasUnUsedAddress) { + boolean hasUnusedAddress = !xmrWalletService.getUnusedAddressEntries().isEmpty(); + if (hasUnusedAddress) { new Popup().warning(Res.get("funds.deposit.selectUnused")).show(); } else { XmrAddressEntry newSavingsAddressEntry = xmrWalletService.getNewAddressEntry(); @@ -311,7 +311,7 @@ public class DepositView extends ActivatableView { // cache incoming txs txsWithIncomingOutputs = xmrWalletService.getTxsWithIncomingOutputs(); - // add available address entries and base address + // add address entries xmrWalletService.getAddressEntries() .forEach(e -> observableList.add(new DepositListItem(e, xmrWalletService, formatter, txsWithIncomingOutputs))); } diff --git a/desktop/src/main/java/haveno/desktop/main/portfolio/openoffer/OpenOffersDataModel.java b/desktop/src/main/java/haveno/desktop/main/portfolio/openoffer/OpenOffersDataModel.java index 80b34748e1..3161bad539 100644 --- a/desktop/src/main/java/haveno/desktop/main/portfolio/openoffer/OpenOffersDataModel.java +++ b/desktop/src/main/java/haveno/desktop/main/portfolio/openoffer/OpenOffersDataModel.java @@ -73,7 +73,7 @@ class OpenOffersDataModel extends ActivatableDataModel { } void onRemoveOpenOffer(OpenOffer openOffer, ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { - openOfferManager.removeOpenOffer(openOffer, resultHandler, errorMessageHandler); + openOfferManager.cancelOpenOffer(openOffer, resultHandler, errorMessageHandler); } diff --git a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep4View.java b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep4View.java index ac82cdd57e..8ed71ec4be 100644 --- a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep4View.java +++ b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep4View.java @@ -121,7 +121,7 @@ public class BuyerStep4View extends TradeStepView { private void handleTradeCompleted() { closeButton.setDisable(true); - model.dataModel.xmrWalletService.swapTradeEntryToAvailableEntry(trade.getId(), XmrAddressEntry.Context.TRADE_PAYOUT); + model.dataModel.xmrWalletService.swapAddressEntryToAvailable(trade.getId(), XmrAddressEntry.Context.TRADE_PAYOUT); openTradeFeedbackWindow(); }