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
This commit is contained in:
woodser 2023-01-18 07:11:01 -05:00
parent 7f26119515
commit b745eaccd4
7 changed files with 51 additions and 14 deletions

View file

@ -13,6 +13,7 @@ import bisq.core.support.dispute.DisputeSummaryVerification;
import bisq.core.support.dispute.arbitration.ArbitrationManager; import bisq.core.support.dispute.arbitration.ArbitrationManager;
import bisq.core.support.messages.ChatMessage; import bisq.core.support.messages.ChatMessage;
import bisq.core.trade.Contract; import bisq.core.trade.Contract;
import bisq.core.trade.HavenoUtils;
import bisq.core.trade.Trade; import bisq.core.trade.Trade;
import bisq.core.trade.TradeManager; import bisq.core.trade.TradeManager;
import bisq.core.util.FormattingUtils; import bisq.core.util.FormattingUtils;
@ -238,6 +239,9 @@ public class CoreDisputesService {
.add(buyerSecurityDeposit)); .add(buyerSecurityDeposit));
} else if (payout == DisputePayout.CUSTOM) { } else if (payout == DisputePayout.CUSTOM) {
Coin winnerAmount = Coin.valueOf(customWinnerAmount); 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); Coin loserAmount = tradeAmount.add(buyerSecurityDeposit).add(sellerSecurityDeposit).minus(winnerAmount);
disputeResult.setBuyerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? winnerAmount : loserAmount); disputeResult.setBuyerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? winnerAmount : loserAmount);
disputeResult.setSellerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? loserAmount : winnerAmount); disputeResult.setSellerPayoutAmount(disputeResult.getWinner() == DisputeResult.Winner.BUYER ? loserAmount : winnerAmount);

View file

@ -85,7 +85,8 @@ public class XmrWalletService {
private static final String MONERO_WALLET_NAME = "haveno_XMR"; private static final String MONERO_WALLET_NAME = "haveno_XMR";
private static final String MONERO_MULTISIG_WALLET_PREFIX = "xmr_multisig_trade_"; 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 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 static final int NUM_MAX_BACKUP_WALLETS = 10;
private final CoreAccountService accountService; private final CoreAccountService accountService;
@ -360,10 +361,10 @@ public class XmrWalletService {
MoneroTxWallet tradeTx = null; MoneroTxWallet tradeTx = null;
double appliedTolerance = 0.0; // percent of tolerance to apply, thereby decreasing security deposit double appliedTolerance = 0.0; // percent of tolerance to apply, thereby decreasing security deposit
double searchDiff = 1.0; // difference for next binary search double searchDiff = 1.0; // difference for next binary search
BigInteger maxAmount = sendAmount.add(securityDeposit);
for (int i = 0; i < 10; i++) { for (int i = 0; i < 10; i++) {
try { 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() tradeTx = wallet.createTx(new MoneroTxConfig()
.setAccountIndex(0) .setAccountIndex(0)
.addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee) .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()); 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); 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); check = wallet.checkTxKey(txHash, txKey, address);
if (!check.isGood()) throw new RuntimeException("Invalid proof of deposit amount"); 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(); BigInteger minSecurityDeposit = new BigDecimal(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 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 { } finally {
try { try {
daemon.flushTxPool(txHash); // flush tx from pool daemon.flushTxPool(txHash); // flush tx from pool

View file

@ -846,6 +846,16 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
BigInteger winnerPayoutAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getBuyerPayoutAmount() : disputeResult.getSellerPayoutAmount()); BigInteger winnerPayoutAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getBuyerPayoutAmount() : disputeResult.getSellerPayoutAmount());
BigInteger loserPayoutAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getSellerPayoutAmount() : disputeResult.getBuyerPayoutAmount()); 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 // create transaction to get fee estimate
MoneroTxConfig txConfig = new MoneroTxConfig().setAccountIndex(0).setRelay(false); 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 (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

View file

@ -329,14 +329,28 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
BigInteger destinationSum = (buyerPayoutDestination == null ? BigInteger.ZERO : buyerPayoutDestination.getAmount()).add(sellerPayoutDestination == null ? BigInteger.ZERO : sellerPayoutDestination.getAmount()); BigInteger destinationSum = (buyerPayoutDestination == null ? BigInteger.ZERO : buyerPayoutDestination.getAmount()).add(sellerPayoutDestination == null ? BigInteger.ZERO : sellerPayoutDestination.getAmount());
if (!arbitratorSignedPayoutTx.getOutputSum().equals(destinationSum.add(arbitratorSignedPayoutTx.getChangeAmount()))) throw new RuntimeException("Sum of outputs != destination amounts + change amount"); if (!arbitratorSignedPayoutTx.getOutputSum().equals(destinationSum.add(arbitratorSignedPayoutTx.getChangeAmount()))) throw new RuntimeException("Sum of outputs != destination amounts + change amount");
// verify winner and loser payout amounts // get actual payout amounts
BigInteger txCost = arbitratorSignedPayoutTx.getFee().add(arbitratorSignedPayoutTx.getChangeAmount()); // fee + lost dust change
BigInteger expectedWinnerAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getBuyerPayoutAmount() : disputeResult.getSellerPayoutAmount());
BigInteger expectedLoserAmount = HavenoUtils.coinToAtomicUnits(disputeResult.getWinner() == Winner.BUYER ? disputeResult.getSellerPayoutAmount() : disputeResult.getBuyerPayoutAmount());
if (expectedLoserAmount.equals(BigInteger.ZERO)) expectedWinnerAmount = expectedWinnerAmount.subtract(txCost); // winner only pays tx cost if loser gets 0
else expectedLoserAmount = expectedLoserAmount.subtract(txCost); // loser pays tx cost
BigInteger actualWinnerAmount = disputeResult.getWinner() == Winner.BUYER ? buyerPayoutDestination.getAmount() : sellerPayoutDestination.getAmount(); BigInteger actualWinnerAmount = disputeResult.getWinner() == Winner.BUYER ? buyerPayoutDestination.getAmount() : sellerPayoutDestination.getAmount();
BigInteger actualLoserAmount = numDestinations == 1 ? BigInteger.ZERO : disputeResult.getWinner() == Winner.BUYER ? sellerPayoutDestination.getAmount() : buyerPayoutDestination.getAmount(); BigInteger actualLoserAmount = numDestinations == 1 ? BigInteger.ZERO : disputeResult.getWinner() == Winner.BUYER ? sellerPayoutDestination.getAmount() : buyerPayoutDestination.getAmount();
// verify payouts sum to unlocked balance within loss of precision due to conversion to centineros
BigInteger txCost = arbitratorSignedPayoutTx.getFee().add(arbitratorSignedPayoutTx.getChangeAmount()); // fee + lost dust change
if (trade.getWallet().getUnlockedBalance().subtract(actualWinnerAmount.add(actualLoserAmount).add(txCost)).compareTo(HavenoUtils.CENTINEROS_AU_MULTIPLIER) > 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 (!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); if (!expectedLoserAmount.equals(actualLoserAmount)) throw new RuntimeException("Unexpected loser payout: " + expectedLoserAmount + " vs " + actualLoserAmount);

View file

@ -61,7 +61,7 @@ public class HavenoUtils {
public static final String LOCALHOST = "localhost"; public static final String LOCALHOST = "localhost";
// multipliers to convert units // 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"); private static BigInteger XMR_AU_MULTIPLIER = new BigInteger("1000000000000");
// TODO: better way to share reference? // TODO: better way to share reference?

View file

@ -27,6 +27,7 @@ import javax.annotation.Nullable;
import bisq.common.app.Version; import bisq.common.app.Version;
import bisq.common.crypto.PubKeyRing; import bisq.common.crypto.PubKeyRing;
import bisq.common.proto.ProtoUtil; import bisq.common.proto.ProtoUtil;
import bisq.common.util.Utilities;
import lombok.EqualsAndHashCode; import lombok.EqualsAndHashCode;
import lombok.Value; import lombok.Value;
@ -86,7 +87,7 @@ public final class DepositsConfirmedMessage extends TradeMailboxMessage {
return "DepositsConfirmedMessage {" + return "DepositsConfirmedMessage {" +
"\n senderNodeAddress=" + senderNodeAddress + "\n senderNodeAddress=" + senderNodeAddress +
",\n pubKeyRing=" + pubKeyRing + ",\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 updatedMultisigHex=" + (updatedMultisigHex == null ? null : updatedMultisigHex.substring(0, Math.max(updatedMultisigHex.length(), 1000))) +
"\n} " + super.toString(); "\n} " + super.toString();
} }

View file

@ -115,6 +115,7 @@ public class GrpcDisputesService extends DisputesImplBase {
responseObserver.onNext(reply); responseObserver.onNext(reply);
responseObserver.onCompleted(); responseObserver.onCompleted();
} catch (Throwable cause) { } catch (Throwable cause) {
cause.printStackTrace();
exceptionHandler.handleExceptionAsWarning(log, getClass().getName() + ".resolveDispute", cause, responseObserver); exceptionHandler.handleExceptionAsWarning(log, getClass().getName() + ".resolveDispute", cause, responseObserver);
} }
} }