From b745eaccd41f7ac1c867273b31e47b88b7c041a0 Mon Sep 17 00:00:00 2001 From: woodser Date: Wed, 18 Jan 2023 07:11:01 -0500 Subject: [PATCH] improve tx verification verify sufficient security deposit which may absorb tx fee payout binary search applies tolerance to security deposit verify payouts sum to wallet balance verify custom winner amount <= wallet balance --- .../bisq/core/api/CoreDisputesService.java | 4 +++ .../core/btc/wallet/XmrWalletService.java | 19 +++++++++----- .../core/support/dispute/DisputeManager.java | 10 +++++++ .../arbitration/ArbitrationManager.java | 26 ++++++++++++++----- .../java/bisq/core/trade/HavenoUtils.java | 2 +- .../messages/DepositsConfirmedMessage.java | 3 ++- .../bisq/daemon/grpc/GrpcDisputesService.java | 1 + 7 files changed, 51 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreDisputesService.java b/core/src/main/java/bisq/core/api/CoreDisputesService.java index 1b3ca1a8f6..a6f036e984 100644 --- a/core/src/main/java/bisq/core/api/CoreDisputesService.java +++ b/core/src/main/java/bisq/core/api/CoreDisputesService.java @@ -13,6 +13,7 @@ import bisq.core.support.dispute.DisputeSummaryVerification; import bisq.core.support.dispute.arbitration.ArbitrationManager; import bisq.core.support.messages.ChatMessage; import bisq.core.trade.Contract; +import bisq.core.trade.HavenoUtils; import bisq.core.trade.Trade; import bisq.core.trade.TradeManager; import bisq.core.util.FormattingUtils; @@ -238,6 +239,9 @@ public class CoreDisputesService { .add(buyerSecurityDeposit)); } else if (payout == DisputePayout.CUSTOM) { Coin winnerAmount = Coin.valueOf(customWinnerAmount); + if (winnerAmount.compareTo(HavenoUtils.atomicUnitsToCoin(trade.getWallet().getBalance())) > 0) { + throw new RuntimeException("The custom winner payout amount is more than the trade wallet's balance"); + } Coin loserAmount = tradeAmount.add(buyerSecurityDeposit).add(sellerSecurityDeposit).minus(winnerAmount); disputeResult.setBuyerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? winnerAmount : loserAmount); disputeResult.setSellerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? loserAmount : winnerAmount); diff --git a/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java b/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java index 11ac7e24f2..32708ad26c 100644 --- a/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java @@ -85,7 +85,8 @@ public class XmrWalletService { private static final String MONERO_WALLET_NAME = "haveno_XMR"; private static final String MONERO_MULTISIG_WALLET_PREFIX = "xmr_multisig_trade_"; 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 absorbs miner fee up to percent + private static final double SECURITY_DEPOSIT_TOLERANCE = Config.baseCurrencyNetwork() == BaseCurrencyNetwork.XMR_LOCAL ? 0.25 : 0.05; // security deposit can abosrb 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 final CoreAccountService accountService; @@ -360,10 +361,10 @@ public class XmrWalletService { 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 - BigInteger maxAmount = sendAmount.add(securityDeposit); for (int i = 0; i < 10; i++) { try { - BigInteger amount = new BigDecimal(maxAmount).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE * appliedTolerance)).toBigInteger(); + BigInteger appliedSecurityDeposit = new BigDecimal(securityDeposit).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE * appliedTolerance)).toBigInteger(); + BigInteger amount = sendAmount.add(appliedSecurityDeposit); tradeTx = wallet.createTx(new MoneroTxConfig() .setAccountIndex(0) .addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee) @@ -434,11 +435,17 @@ public class XmrWalletService { if (feeDiff > MINER_FEE_TOLERANCE) throw new Error("Miner fee is not within " + (MINER_FEE_TOLERANCE * 100) + "% of estimated fee, expected " + feeEstimate + " but was " + tx.getFee()); log.info("Trade tx fee {} is within tolerance, diff%={}", tx.getFee(), feeDiff); - // verify deposit amount + // verify sufficient security deposit check = wallet.checkTxKey(txHash, txKey, address); if (!check.isGood()) throw new RuntimeException("Invalid proof of deposit amount"); - BigInteger minAmount = new BigDecimal(sendAmount.add(securityDeposit)).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE)).toBigInteger(); - if (check.getReceivedAmount().compareTo(minAmount) < 0) throw new RuntimeException("Deposit amount is not enough, needed " + minAmount + " but was " + check.getReceivedAmount()); + BigInteger minSecurityDeposit = new BigDecimal(securityDeposit).multiply(new BigDecimal(1.0 - SECURITY_DEPOSIT_TOLERANCE)).toBigInteger(); + BigInteger actualSecurityDeposit = check.getReceivedAmount().subtract(sendAmount); + if (actualSecurityDeposit.compareTo(minSecurityDeposit) < 0) throw new RuntimeException("Security deposit amount is not enough, needed " + minSecurityDeposit + " but was " + actualSecurityDeposit); + + // 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()); + BigInteger actualDepositAndFee = check.getReceivedAmount().add(tx.getFee()); + if (actualDepositAndFee.compareTo(minDepositAndFee) < 0) throw new RuntimeException("Deposit amount + fee is not enough, needed " + minDepositAndFee + " but was " + actualDepositAndFee); } finally { try { daemon.flushTxPool(txHash); // flush tx from pool diff --git a/core/src/main/java/bisq/core/support/dispute/DisputeManager.java b/core/src/main/java/bisq/core/support/dispute/DisputeManager.java index b81e7cff28..c5072efc52 100644 --- a/core/src/main/java/bisq/core/support/dispute/DisputeManager.java +++ b/core/src/main/java/bisq/core/support/dispute/DisputeManager.java @@ -846,6 +846,16 @@ public abstract class DisputeManager> extends Sup BigInteger winnerPayoutAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getBuyerPayoutAmount() : disputeResult.getSellerPayoutAmount()); BigInteger loserPayoutAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getSellerPayoutAmount() : disputeResult.getBuyerPayoutAmount()); + // check sufficient balance + 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"); + } + + // 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 diff --git a/core/src/main/java/bisq/core/support/dispute/arbitration/ArbitrationManager.java b/core/src/main/java/bisq/core/support/dispute/arbitration/ArbitrationManager.java index 7188fcd4a3..384a0cfcf9 100644 --- a/core/src/main/java/bisq/core/support/dispute/arbitration/ArbitrationManager.java +++ b/core/src/main/java/bisq/core/support/dispute/arbitration/ArbitrationManager.java @@ -329,14 +329,28 @@ public final class ArbitrationManager extends DisputeManager 0) { + throw new RuntimeException("The dispute payout amounts do not sum to the wallet's unlocked balance while verifying the dispute payout tx, unlocked balance=" + trade.getWallet().getUnlockedBalance() + " vs sum payout amount=" + actualWinnerAmount.add(actualLoserAmount) + ", winner payout=" + actualWinnerAmount + ", loser payout=" + actualLoserAmount); + } + + // get expected payout amounts + BigInteger expectedWinnerAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getBuyerPayoutAmount() : disputeResult.getSellerPayoutAmount()); + BigInteger expectedLoserAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getSellerPayoutAmount() : disputeResult.getBuyerPayoutAmount()); + + // add any loss of precision to winner amount + expectedWinnerAmount = expectedWinnerAmount.add(trade.getWallet().getUnlockedBalance().subtract(expectedWinnerAmount.add(expectedLoserAmount))); + + // winner pays cost if loser gets nothing, otherwise loser pays cost + if (expectedLoserAmount.equals(BigInteger.ZERO)) expectedWinnerAmount = expectedWinnerAmount.subtract(txCost); + else expectedLoserAmount = expectedLoserAmount.subtract(txCost); + + // verify winner and loser payout amounts if (!expectedWinnerAmount.equals(actualWinnerAmount)) throw new RuntimeException("Unexpected winner payout: " + expectedWinnerAmount + " vs " + actualWinnerAmount); if (!expectedLoserAmount.equals(actualLoserAmount)) throw new RuntimeException("Unexpected loser payout: " + expectedLoserAmount + " vs " + actualLoserAmount); diff --git a/core/src/main/java/bisq/core/trade/HavenoUtils.java b/core/src/main/java/bisq/core/trade/HavenoUtils.java index d46b902457..70bbc1373e 100644 --- a/core/src/main/java/bisq/core/trade/HavenoUtils.java +++ b/core/src/main/java/bisq/core/trade/HavenoUtils.java @@ -61,7 +61,7 @@ public class HavenoUtils { public static final String LOCALHOST = "localhost"; // multipliers to convert units - private static BigInteger CENTINEROS_AU_MULTIPLIER = new BigInteger("10000"); + public static BigInteger CENTINEROS_AU_MULTIPLIER = new BigInteger("10000"); private static BigInteger XMR_AU_MULTIPLIER = new BigInteger("1000000000000"); // TODO: better way to share reference? diff --git a/core/src/main/java/bisq/core/trade/messages/DepositsConfirmedMessage.java b/core/src/main/java/bisq/core/trade/messages/DepositsConfirmedMessage.java index 9d81305a40..f6fd61196f 100644 --- a/core/src/main/java/bisq/core/trade/messages/DepositsConfirmedMessage.java +++ b/core/src/main/java/bisq/core/trade/messages/DepositsConfirmedMessage.java @@ -27,6 +27,7 @@ import javax.annotation.Nullable; import bisq.common.app.Version; import bisq.common.crypto.PubKeyRing; import bisq.common.proto.ProtoUtil; +import bisq.common.util.Utilities; import lombok.EqualsAndHashCode; import lombok.Value; @@ -86,7 +87,7 @@ public final class DepositsConfirmedMessage extends TradeMailboxMessage { return "DepositsConfirmedMessage {" + "\n senderNodeAddress=" + senderNodeAddress + ",\n pubKeyRing=" + pubKeyRing + - ",\n sellerPaymentAccountKey=" + sellerPaymentAccountKey + + ",\n sellerPaymentAccountKey=" + Utilities.bytesAsHexString(sellerPaymentAccountKey) + ",\n updatedMultisigHex=" + (updatedMultisigHex == null ? null : updatedMultisigHex.substring(0, Math.max(updatedMultisigHex.length(), 1000))) + "\n} " + super.toString(); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputesService.java index eaa7f1fd58..82de58a732 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputesService.java @@ -115,6 +115,7 @@ public class GrpcDisputesService extends DisputesImplBase { responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { + cause.printStackTrace(); exceptionHandler.handleExceptionAsWarning(log, getClass().getName() + ".resolveDispute", cause, responseObserver); } }