Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add signature to dispute result and various other improvements #4543

Merged
merged 42 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4f1cbbd
Add check for refund agent if donation address is valid
chimp1984 Sep 11, 2020
2b04338
Dont allow opening refudn agent dispute if delayed payout tx is invalid.
chimp1984 Sep 11, 2020
c48abbf
Improve address validation code
chimp1984 Sep 11, 2020
d82631f
Fix some issues found during testing
chimp1984 Sep 12, 2020
677211b
Allow close dispute for refund agent without payout
chimp1984 Sep 12, 2020
08fb596
Call validatePayoutTx only after trades are initialized
chimp1984 Sep 12, 2020
05e1039
Call validatePayoutTx only after trades are initialized
chimp1984 Sep 12, 2020
7ac6e71
Dispute agent sign summary. Add tool for verification
chimp1984 Sep 12, 2020
559028e
Remove unused var
chimp1984 Sep 12, 2020
48066ae
Remove setting of pubKey as it is not needed
chimp1984 Sep 12, 2020
0c46e7d
Add more data to summary msg
chimp1984 Sep 12, 2020
de4fb17
Improve summary notes
chimp1984 Sep 13, 2020
966b22a
Fix line breaks
chimp1984 Sep 13, 2020
29f3a7c
Merge branch 'master_upstream' into allow-refund-agent-close-without-…
chimp1984 Sep 17, 2020
b0b4334
Merge branch 'master_upstream' into dispute-agents-sign-summary
chimp1984 Sep 17, 2020
1c0bef7
Merge branch 'master_upstream' into verify-donation-address-for-refun…
chimp1984 Sep 17, 2020
1c41db4
Fix wrong handling of mainnet RECIPIENT_BTC_ADDRESSes
chimp1984 Sep 17, 2020
3d4427c
Add result of filter match. Add more filter data (tx ids, json)
chimp1984 Sep 17, 2020
45cee2a
Add check for disputes with duplicated trade ID or payout tx ids
chimp1984 Sep 18, 2020
f46a991
Merge branch 'dispute-agents-sign-summary' into dispute-agent-branch
chimp1984 Sep 18, 2020
b2a9262
Merge branch 'verify-donation-address-for-refund-agent' into dispute-…
chimp1984 Sep 18, 2020
c1850cb
Merge branch 'master_upstream' into dispute-agent-branch
chimp1984 Sep 20, 2020
3293047
Set agentsUid to new uuid in case it is null from persisted data
chimp1984 Sep 20, 2020
d31deff
Remove dev log
chimp1984 Sep 20, 2020
25bc616
Add check for multiple deposit txs
chimp1984 Sep 20, 2020
4878a10
Optimize testIfDisputeTriesReplay methods to avoid that maps get crea…
chimp1984 Sep 20, 2020
c6778d6
Add copy to csv data button to report screen
chimp1984 Sep 21, 2020
72dca0b
Add cylce index
chimp1984 Sep 21, 2020
2943316
Remove agentsUid from protobuf, rename to uid
chimp1984 Sep 21, 2020
30e9add
Refactor: rename DelayedPayoutTxValidation to TradeDataValidation
chimp1984 Sep 21, 2020
3206c62
Refactor: Move RegexValidator from bisq.desktop.util.validation to bi…
chimp1984 Sep 21, 2020
987bf49
Add node address validation
chimp1984 Sep 21, 2020
baa915f
Add validateNodeAddress at onOpenNewDisputeMessage
chimp1984 Sep 21, 2020
d52199e
Merge branch 'refactor-regexvalidator' into dispute-agent-branch
chimp1984 Sep 21, 2020
c7a3f95
Rename filterString to filterTerm
chimp1984 Sep 21, 2020
76c8263
Ignore onion address validation for localhost
chimp1984 Sep 21, 2020
a9f1062
Move validation after adding dispute to list
chimp1984 Sep 21, 2020
81bea14
Show popup to peer who accepted mediators suggestion once locktime is…
chimp1984 Sep 21, 2020
f37446b
Change log level
chimp1984 Sep 25, 2020
8ac468d
Commit to trigger travis as it got stuck...
chimp1984 Sep 25, 2020
25c4b4d
Merge branch 'master_upstream' into dispute-agent-branch
chimp1984 Sep 25, 2020
423cc71
Add changes from merge conflict (class was renamed)
chimp1984 Sep 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,9 @@ protected void onOpenNewDisputeMessage(OpenNewDisputeMessage openNewDisputeMessa
Contract contract = dispute.getContract();
addPriceInfoMessage(dispute, 0);

if (!DelayedPayoutTxValidation.isValidDonationAddress(dispute, daoFacade)) {
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);
} catch (DelayedPayoutTxValidation.DonationAddressException e) {
disputesWithInvalidDonationAddress.add(dispute);
}

Expand Down
75 changes: 22 additions & 53 deletions core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import bisq.core.offer.Offer;
import bisq.core.support.dispute.Dispute;

import bisq.common.config.Config;

import org.bitcoinj.core.Address;
import org.bitcoinj.core.Coin;
import org.bitcoinj.core.NetworkParameters;
Expand All @@ -33,7 +31,6 @@
import org.bitcoinj.core.TransactionOutPoint;
import org.bitcoinj.core.TransactionOutput;

import java.util.List;
import java.util.Set;
import java.util.function.Consumer;

Expand Down Expand Up @@ -83,26 +80,33 @@ public static class InvalidInputException extends Exception {
}
}

public static boolean isValidDonationAddress(Dispute dispute, DaoFacade daoFacade) {
String addressAsString = dispute.getDonationAddressOfDelayedPayoutTx();
public static void validateDonationAddress(String addressAsString, DaoFacade daoFacade)
throws DonationAddressException {

// Old clients don't have it set yet. Can be removed after a forced update
if (addressAsString == null) {
return true;
return;
}

// We use all past addresses from DAO param changes as the dispute case might have been opened later and the
// DAO param changed in the meantime.
// We support any of the past addresses as well as in case the peer has not enabled the DAO or is out of sync we
// do not want to break validation.
Set<String> allPastParamValues = daoFacade.getAllPastParamValues(Param.RECIPIENT_BTC_ADDRESS);

if (allPastParamValues.contains(addressAsString)) {
return true;
}
// If Dao is deactivated we need to add the default address as getAllPastParamValues will not return us any.
allPastParamValues.add(Param.RECIPIENT_BTC_ADDRESS.getDefaultValue());

// If Dao is deactivated we need to add the past addresses used as well.
// This list need to be updated once a new address gets defined.
allPastParamValues.add("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"); // burning man 2019
allPastParamValues.add("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2
chimp1984 marked this conversation as resolved.
Show resolved Hide resolved


log.warn("Donation address is not a valid DAO donation address." +
"\nAddress used in the dispute: " + addressAsString +
"\nAll DAO param donation addresses:" + allPastParamValues);
return false;
if (!allPastParamValues.contains(addressAsString)) {
String errorMsg = "Donation address is not a valid DAO donation address." +
"\nAddress used in the dispute: " + addressAsString +
"\nAll DAO param donation addresses:" + allPastParamValues;
log.error(errorMsg);
throw new DonationAddressException(errorMsg);
}
}

public static void validatePayoutTx(Trade trade,
Expand Down Expand Up @@ -212,16 +216,10 @@ public static void validatePayoutTx(Trade trade,
throw new AmountMismatchException(errorMsg);
}


// Validate donation address
// Get most recent donation address.
// We do not support past DAO param addresses to avoid that those receive funds (no bond set up anymore).
// Users who have not synced the DAO cannot trade.

NetworkParameters params = btcWalletService.getParams();
Address address = output.getAddressFromP2PKHScript(params);
if (address == null) {
// The donation address can be as well be a multisig address.
// The donation address can be a multisig address as well.
address = output.getAddressFromP2SH(params);
if (address == null) {
errorMsg = "Donation address cannot be resolved (not of type P2PKHScript or P2SH). Output: " + output;
Expand All @@ -236,36 +234,7 @@ public static void validatePayoutTx(Trade trade,
addressConsumer.accept(addressAsString);
}

// In case the seller has deactivated the DAO the default address will be used.
String defaultDonationAddressString = Param.RECIPIENT_BTC_ADDRESS.getDefaultValue();
boolean defaultNotMatching = !defaultDonationAddressString.equals(addressAsString);
String recentDonationAddressString = daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS);
boolean recentFromDaoNotMatching = !recentDonationAddressString.equals(addressAsString);

// If buyer has DAO deactivated or not synced he will not be able to see recent address used by the seller, so
// we add it hard coded here. We need to support also the default one as
// FIXME This is a quick fix and should be improved in future.
// We use the default addresses for non mainnet networks. For dev testing it need to be changed here.
// We use a list to gain more flexibility at updates of DAO param, but still might fail if buyer has not updated
// software. Needs a better solution....
List<String> hardCodedAddresses = Config.baseCurrencyNetwork().isMainnet() ?
List.of("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp", "3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV") : // mainnet
Config.baseCurrencyNetwork().isDaoBetaNet() ? List.of("1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7") : // daoBetaNet
Config.baseCurrencyNetwork().isTestnet() ? List.of("2N4mVTpUZAnhm9phnxB7VrHB4aBhnWrcUrV") : // testnet
List.of("2MzBNTJDjjXgViKBGnatDU3yWkJ8pJkEg9w"); // regtest or DAO testnet (regtest)

boolean noneOfHardCodedMatching = hardCodedAddresses.stream().noneMatch(e -> e.equals(addressAsString));

// If seller has DAO deactivated as well we get default address
if (recentFromDaoNotMatching && defaultNotMatching && noneOfHardCodedMatching) {
errorMsg = "Donation address is invalid." +
"\nAddress used by BTC seller: " + addressAsString +
"\nRecent donation address:" + recentDonationAddressString +
"\nDefault donation address: " + defaultDonationAddressString;
log.error(errorMsg);
log.error(delayedPayoutTx.toString());
throw new DonationAddressException(errorMsg);
}
validateDonationAddress(addressAsString, daoFacade);

if (dispute != null) {
// Verify that address in the dispute matches the one in the trade.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,17 +650,18 @@ private void addButtons(Contract contract) {
return;
}

if (!DelayedPayoutTxValidation.isValidDonationAddress(dispute, daoFacade)) {
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);

if (dispute.getSupportType() == SupportType.REFUND &&
peersDisputeOptional.isPresent() &&
!peersDisputeOptional.get().isClosed()) {
showPayoutTxConfirmation(contract, disputeResult, () -> doClose(closeTicketButton));
} else {
doClose(closeTicketButton);
}
} catch (DelayedPayoutTxValidation.DonationAddressException exception) {
showInValidDonationAddressPopup();
} else if (dispute.getSupportType() == SupportType.REFUND &&
peersDisputeOptional.isPresent() &&
!peersDisputeOptional.get().isClosed()) {
showPayoutTxConfirmation(contract, disputeResult,
() -> {
doClose(closeTicketButton);
});
} else {
doClose(closeTicketButton);
}
});

Expand Down Expand Up @@ -752,7 +753,9 @@ public void onFailure(TxBroadcastException exception) {

private void doClose(Button closeTicketButton) {
var disputeManager = checkNotNull(getDisputeManager(dispute));
if (!DelayedPayoutTxValidation.isValidDonationAddress(dispute, daoFacade)) {
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);
} catch (DelayedPayoutTxValidation.DonationAddressException exception) {
showInValidDonationAddressPopup();

// For mediators we do not enforce that the case cannot be closed to stay flexible,
Expand Down