Skip to content

Commit

Permalink
Adapt refund agent tx chain validation to v5 protocol
Browse files Browse the repository at this point in the history 
Make sure the logic to fetch the chain of trade txs from mempool.space,
and validate the receiver list, works in the case of the v5 protocol. In
that case, the published warning & redirect txs replace the DPT, giving
five txs in the chain instead of four. Ensure that in both cases the
number of confirmations of the last tx (DPT for v4, redirect tx for v5)
shows up in Dispute Summary window (outside of regtest). Also fix a bug
validating the outputs of the last tx, where it failed to iterate
through the receiver tuples correctly, which would have prevented it
from ever working in production (as there's more than one receiver).

Finally, make the dispute validation more strict about which txId fields
it allows to be null vs non-null, so that the DPT ID is always null when
the redirect or warning txId fields are set (signifying a v5 protocol
trade for refund disputes). Disallow use of the legacy BM in that case
as well, for safety, as it should never be used for new trades.
  • Loading branch information
@stejbac
stejbac committed Oct 10, 2024
1 parent da05ebf commit 360b8a4
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 119 deletions.
54 changes: 31 additions & 23 deletions core/src/main/java/bisq/core/support/dispute/DisputeValidation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ public static void validateTradeAndDispute(Dispute dispute, Trade trade, BtcWall
checkArgument(dispute.getContract().equals(trade.getContract()),
"contract must match contract from trade");

if (trade.hasV5Protocol()) {
if (!trade.hasV5Protocol()) {
checkNotNull(trade.getDelayedPayoutTx(), "trade.getDelayedPayoutTx() must not be null");
checkNotNull(dispute.getDelayedPayoutTxId(), "delayedPayoutTxId must not be null");
checkArgument(dispute.getDelayedPayoutTxId().equals(trade.getDelayedPayoutTx().getTxId().toString()),
"delayedPayoutTxId must match delayedPayoutTxId from trade");
} else if (dispute.getSupportType() == SupportType.REFUND) {
String buyersWarningTxId = toTxId(trade.getBuyersWarningTx(btcWalletService));
String sellersWarningTxId = toTxId(trade.getSellersWarningTx(btcWalletService));
String buyersRedirectTxId = toTxId(trade.getBuyersRedirectTx(btcWalletService));
Expand All @@ -123,15 +128,12 @@ public static void validateTradeAndDispute(Dispute dispute, Trade trade, BtcWall
checkArgument(isBuyerRedirect, "seller's redirectTx must be used with buyer's warningTx");
}
} else {
checkNotNull(trade.getDelayedPayoutTx(), "trade.getDelayedPayoutTx() must not be null");
checkNotNull(dispute.getDelayedPayoutTxId(), "delayedPayoutTxId must not be null");
checkArgument(dispute.getDelayedPayoutTxId().equals(trade.getDelayedPayoutTx().getTxId().toString()),
"delayedPayoutTxId must match delayedPayoutTxId from trade");
checkArgument(dispute.getDelayedPayoutTxId() == null, "delayedPayoutTxId should be null");
}

checkTxIdFieldCombination(dispute);
checkNotNull(trade.getDepositTx(), "trade.getDepositTx() must not be null");
checkNotNull(dispute.getDepositTxId(), "depositTxId must not be null");
checkArgument(dispute.getDepositTxId().equals(trade.getDepositTx().getTxId().toString()),
checkArgument(trade.getDepositTx().getTxId().toString().equals(dispute.getDepositTxId()),
"depositTx must match depositTx from trade");

checkNotNull(dispute.getDepositTxSerialized(), "depositTxSerialized must not be null");
Expand Down Expand Up @@ -245,22 +247,7 @@ private static void testIfDisputeTriesReplay(Dispute disputeToTest,
try {
String disputeToTestTradeId = disputeToTest.getTradeId();

// For pre v1.4.0 we do not get the delayed payout tx sent in mediation cases but in refund agent case we do.
// With 1.4.0 we send the delayed payout tx also in mediation cases. For v5 protocol trades, there is no DPT
// and it is unknown which staged txs will be published, if any, so they are only sent in refund agent cases.
if (disputeToTest.getSupportType() == SupportType.REFUND) {
if (disputeToTest.getWarningTxId() == null) {
checkNotNull(disputeToTest.getDelayedPayoutTxId(),
"Delayed payout transaction ID is null. " +
"Trade ID: %s", disputeToTestTradeId);
} else {
checkNotNull(disputeToTest.getRedirectTxId(),
"Redirect transaction ID is null. " +
"Trade ID: %s", disputeToTestTradeId);
}
}
checkNotNull(disputeToTest.getDepositTxId(),
"depositTxId must not be null. Trade ID: %s", disputeToTestTradeId);
checkTxIdFieldCombination(disputeToTest);
checkNotNull(disputeToTest.getUid(),
"agentsUid must not be null. Trade ID: %s", disputeToTestTradeId);

Expand All @@ -280,6 +267,27 @@ private static void testIfDisputeTriesReplay(Dispute disputeToTest,
}
}

private static void checkTxIdFieldCombination(Dispute dispute) {
// For pre v1.4.0 we do not get the delayed payout tx sent in mediation cases but in refund agent case we do.
// With 1.4.0 we send the delayed payout tx also in mediation cases. For v5 protocol trades, there is no DPT
// and it is unknown which staged txs will be published, if any, so they are only sent in refund agent cases.
String tradeId = dispute.getTradeId();
if (dispute.getWarningTxId() != null || dispute.getRedirectTxId() != null) {
checkNotNull(dispute.getWarningTxId(), "warningTxId must not be null. Trade ID: %s", tradeId);
checkNotNull(dispute.getRedirectTxId(), "redirectTxId must not be null. Trade ID: %s", tradeId);
checkArgument(dispute.getDelayedPayoutTxId() == null,
"delayedPayoutTxId should be null. Trade ID: %s", tradeId);
checkArgument(!dispute.isUsingLegacyBurningMan(),
"Legacy BM use not permitted. Trade ID: %s", tradeId);
checkArgument(dispute.getSupportType() == SupportType.REFUND,
"Mediation not permitted after staged txs published. Trade ID: %s", tradeId);
} else if (dispute.getSupportType() == SupportType.REFUND) {
checkNotNull(dispute.getDelayedPayoutTxId(),
"delayedPayoutTxId must not be null. Trade ID: %s", tradeId);
}
checkNotNull(dispute.getDepositTxId(), "depositTxId must not be null. Trade ID: %s", tradeId);
}

private enum DisputeIdField implements Function<Dispute, String> {
TRADE_ID(Dispute::getTradeId),
DELAYED_PAYOUT_TX_ID(Dispute::getDelayedPayoutTxId),
Expand Down
124 changes: 85 additions & 39 deletions core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import bisq.core.trade.TradeManager;
import bisq.core.trade.bisq_v1.FailedTradesManager;
import bisq.core.trade.model.bisq_v1.Trade;
import bisq.core.trade.protocol.bisq_v5.model.StagedPayoutTxParameters;

import bisq.network.p2p.AckMessageSourceType;
import bisq.network.p2p.NodeAddress;
Expand All @@ -64,6 +65,7 @@
import com.google.inject.Singleton;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -262,10 +264,7 @@ public NodeAddress getAgentNodeAddress(Dispute dispute) {
return dispute.getContract().getRefundAgentNodeAddress();
}

public CompletableFuture<List<Transaction>> requestBlockchainTransactions(String makerFeeTxId,
String takerFeeTxId,
String depositTxId,
String delayedPayoutTxId) {
public CompletableFuture<List<Transaction>> requestBlockchainTransactions(List<String> txIds) {
// in regtest mode, simulate a delay & failure obtaining the blockchain transactions
// since we cannot request them in regtest anyway. this is useful for checking failure scenarios
if (!Config.baseCurrencyNetwork().isMainnet()) {
Expand All @@ -276,28 +275,28 @@ public CompletableFuture<List<Transaction>> requestBlockchainTransactions(String

NetworkParameters params = btcWalletService.getParams();
List<Transaction> txs = new ArrayList<>();
return mempoolService.requestTxAsHex(makerFeeTxId)
.thenCompose(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return mempoolService.requestTxAsHex(takerFeeTxId);
}).thenCompose(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return mempoolService.requestTxAsHex(depositTxId);
}).thenCompose(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return mempoolService.requestTxAsHex(delayedPayoutTxId);
})
.thenApply(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return txs;
});
Iterator<String> txIdIterator = txIds.iterator();
if (!txIdIterator.hasNext()) {
return CompletableFuture.completedFuture(txs);
}
CompletableFuture<String> future = mempoolService.requestTxAsHex(txIdIterator.next());
while (txIdIterator.hasNext()) {
String txId = txIdIterator.next();
future = future.thenCompose(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return mempoolService.requestTxAsHex(txId);
});
}
return future.thenApply(txAsHex -> {
txs.add(new Transaction(params, Hex.decode(txAsHex)));
return txs;
});
}

public void verifyTradeTxChain(List<Transaction> txs) {
Transaction makerFeeTx = txs.get(0);
Transaction takerFeeTx = txs.get(1);
Transaction depositTx = txs.get(2);
Transaction delayedPayoutTx = txs.get(3);

// The order and number of buyer and seller inputs are not part of the trade protocol consensus.
// In the current implementation buyer inputs come before seller inputs at depositTx and there is
Expand All @@ -316,38 +315,85 @@ public void verifyTradeTxChain(List<Transaction> txs) {
}
checkArgument(makerFeeTxFoundAtInputs, "makerFeeTx not found at depositTx inputs");
checkArgument(takerFeeTxFoundAtInputs, "takerFeeTx not found at depositTx inputs");
checkArgument(depositTx.getInputs().size() >= 2,
"DepositTx must have at least 2 inputs");
checkArgument(delayedPayoutTx.getInputs().size() == 1,
"DelayedPayoutTx must have 1 input");
TransactionOutPoint delayedPayoutTxInputOutpoint = delayedPayoutTx.getInputs().get(0).getOutpoint();
String fundingTxId = delayedPayoutTxInputOutpoint.getHash().toString();
checkArgument(fundingTxId.equals(depositTx.getTxId().toString()),
"First input at delayedPayoutTx does not connect to depositTx");
checkArgument(depositTx.getInputs().size() >= 2, "depositTx must have at least 2 inputs");
if (txs.size() == 4) {
Transaction delayedPayoutTx = txs.get(3);
checkArgument(delayedPayoutTx.getInputs().size() == 1, "delayedPayoutTx must have 1 input");
checkArgument(firstOutputConnectsToFirstInput(depositTx, delayedPayoutTx),
"First input at delayedPayoutTx does not connect to depositTx");
} else {
Transaction warningTx = txs.get(3);
Transaction redirectTx = txs.get(4);

checkArgument(warningTx.getInputs().size() == 1, "warningTx must have 1 input");
checkArgument(warningTx.getOutputs().size() == 2, "warningTx must have 2 outputs");
checkArgument(warningTx.getOutput(1).getValue().value ==
StagedPayoutTxParameters.WARNING_TX_FEE_BUMP_OUTPUT_VALUE,
"Second warningTx output is wrong amount for a fee bump output");

checkArgument(redirectTx.getInputs().size() == 1, "redirectTx must have 1 input");
int numReceivers = redirectTx.getOutputs().size() - 1;
checkArgument(redirectTx.getOutput(numReceivers).getValue().value ==
StagedPayoutTxParameters.REDIRECT_TX_FEE_BUMP_OUTPUT_VALUE,
"Last redirectTx output is wrong amount for a fee bump output");

checkArgument(firstOutputConnectsToFirstInput(depositTx, warningTx),
"First input at warningTx does not connect to depositTx");
checkArgument(firstOutputConnectsToFirstInput(warningTx, redirectTx),
"First input at redirectTx does not connect to warningTx");
}
}

public void verifyDelayedPayoutTxReceivers(Transaction delayedPayoutTx, Dispute dispute) {
Transaction depositTx = dispute.findDepositTx(btcWalletService).orElseThrow();
private static boolean firstOutputConnectsToFirstInput(Transaction parent, Transaction child) {
TransactionOutPoint childTxInputOutpoint = child.getInput(0).getOutpoint();
String fundingTxId = childTxInputOutpoint.getHash().toString();
return fundingTxId.equals(parent.getTxId().toString());
}

public void verifyDelayedPayoutTxReceivers(Transaction depositTx, Transaction delayedPayoutTx, Dispute dispute) {
long inputAmount = depositTx.getOutput(0).getValue().value;
int selectionHeight = dispute.getBurningManSelectionHeight();

List<Tuple2<Long, String>> delayedPayoutTxReceivers = delayedPayoutTxReceiverService.getReceivers(
List<Tuple2<Long, String>> receivers = delayedPayoutTxReceiverService.getReceivers(
selectionHeight,
inputAmount,
dispute.getTradeTxFee(),
DelayedPayoutTxReceiverService.ReceiverFlag.flagsActivatedBy(dispute.getTradeDate()));
log.info("Verify delayedPayoutTx using selectionHeight {} and receivers {}", selectionHeight, delayedPayoutTxReceivers);
checkArgument(delayedPayoutTx.getOutputs().size() == delayedPayoutTxReceivers.size(),
"Size of outputs and delayedPayoutTxReceivers must be the same");
log.info("Verify delayedPayoutTx using selectionHeight {} and receivers {}", selectionHeight, receivers);
checkArgument(delayedPayoutTx.getOutputs().size() == receivers.size(),
"Number of outputs must equal number of receivers");
checkOutputsPrefixMatchesReceivers(delayedPayoutTx, receivers);
}

public void verifyRedirectTxReceivers(Transaction warningTx, Transaction redirectTx, Dispute dispute) {
long inputAmount = warningTx.getOutput(0).getValue().value;
long inputAmountMinusFeeBumpAmount = inputAmount - StagedPayoutTxParameters.REDIRECT_TX_FEE_BUMP_OUTPUT_VALUE;
int selectionHeight = dispute.getBurningManSelectionHeight();

List<Tuple2<Long, String>> receivers = delayedPayoutTxReceiverService.getReceivers(
selectionHeight,
inputAmountMinusFeeBumpAmount,
dispute.getTradeTxFee(),
StagedPayoutTxParameters.REDIRECT_TX_MIN_WEIGHT,
DelayedPayoutTxReceiverService.ReceiverFlag.flagsActivatedBy(dispute.getTradeDate()));
log.info("Verify redirectTx using selectionHeight {} and receivers {}", selectionHeight, receivers);
checkArgument(redirectTx.getOutputs().size() == receivers.size() + 1,
"Number of outputs must equal number of receivers plus 1");
checkOutputsPrefixMatchesReceivers(redirectTx, receivers);
}

private void checkOutputsPrefixMatchesReceivers(Transaction delayedPayoutOrRedirectTx,
List<Tuple2<Long, String>> receivers) {
NetworkParameters params = btcWalletService.getParams();
for (int i = 0; i < delayedPayoutTx.getOutputs().size(); i++) {
TransactionOutput transactionOutput = delayedPayoutTx.getOutputs().get(i);
Tuple2<Long, String> receiverTuple = delayedPayoutTxReceivers.get(0);
for (int i = 0; i < receivers.size(); i++) {
TransactionOutput transactionOutput = delayedPayoutOrRedirectTx.getOutput(i);
Tuple2<Long, String> receiverTuple = receivers.get(i);
checkArgument(transactionOutput.getScriptPubKey().getToAddress(params).toString().equals(receiverTuple.second),
"output address does not match delayedPayoutTxReceivers address. transactionOutput=" + transactionOutput);
"Output address does not match receiver address (%s). transactionOutput=%s",
receiverTuple.second, transactionOutput);
checkArgument(transactionOutput.getValue().value == receiverTuple.first,
"output value does not match delayedPayoutTxReceivers value. transactionOutput=" + transactionOutput);
"Output value does not match receiver value (%s). transactionOutput=%s",
receiverTuple.first, transactionOutput);
}
}
}
4 changes: 2 additions & 2 deletions core/src/main/resources/i18n/displayStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2997,9 +2997,9 @@ disputeSummaryWindow.tradePeriodEnd=Trade period end
disputeSummaryWindow.extraInfo=Extra information
disputeSummaryWindow.delayedPayoutStatus=Delayed Payout Status
disputeSummaryWindow.requestingTxs=Requesting blockchain transactions from block explorer...
disputeSummaryWindow.requestTransactionsError=Requesting the 4 trade transactions failed. Error message: {0}.\n\n\
disputeSummaryWindow.requestTransactionsError=Requesting the {0} trade transactions failed. Error message: {1}.\n\n\
Please verify the transactions manually before closing the dispute.
disputeSummaryWindow.delayedPayoutTxVerificationFailed=Verification of the delayed payout transaction failed. Error message: {0}.\n\n\
disputeSummaryWindow.delayedPayoutTxVerificationFailed=Verification of the delayed payout / redirection transaction failed. Error message: {0}.\n\n\
Please do not make the payout but get in touch with developers to clarify the case.

# dynamic values are not recognized by IntelliJ
Expand Down
Loading

0 comments on commit 360b8a4

Please sign in to comment.