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

[WIP] Add cancel trade feature #4514

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
59860f6
Refactor: Rename method
chimp1984 Sep 10, 2020
3ef7599
Refactor: Move initialize before activate, add separator lines
chimp1984 Sep 10, 2020
2bfdcfb
Refactor: Move methods
chimp1984 Sep 10, 2020
1159836
Refactor: Make initialize public
chimp1984 Sep 10, 2020
59e4e98
Functional change: Do not call initialize in constructor
chimp1984 Sep 10, 2020
17ba6e9
Refactor: Do check for trade.getPayoutTx() == null at beginning and r…
chimp1984 Sep 10, 2020
1e2ea6d
Refactor: Move check for trade.getPayoutTx() == null to sub classes
chimp1984 Sep 10, 2020
88cd5b2
Refactor: Rename class for more generic use cases.
chimp1984 Sep 10, 2020
5f1d5f3
Functional change: Avoid possibility to add duplicated tradable to list
chimp1984 Sep 10, 2020
1f2241f
Add cancel trade domain
chimp1984 Sep 10, 2020
5dd81cb
Refactor: Rename id to tradeId
chimp1984 Sep 10, 2020
d50d825
Remove unused field
chimp1984 Sep 10, 2020
25db775
Refactor: Rename CanceledTradeState to HandleCancelTradeRequestState
chimp1984 Sep 10, 2020
68add41
Add RequestCancelTradeState
chimp1984 Sep 10, 2020
b28f507
Separate states completed
chimp1984 Sep 10, 2020
8236ce2
Remove cancel support at step 3. Once buyer has send payment we do
chimp1984 Sep 10, 2020
6f5e386
Rename RequestCancelTradePresentation and HandleCancelTradeRequestPre…
chimp1984 Sep 10, 2020
79a9ac9
Refactor: Move classes
chimp1984 Sep 10, 2020
7313458
Refactor: Rename enums
chimp1984 Sep 10, 2020
e483efd
Refactor: Move enums to seller and buyer trade
chimp1984 Sep 10, 2020
3e85ff6
Refactor: Rename enums
chimp1984 Sep 10, 2020
a134c74
Split buyer seller protocol classes.
chimp1984 Sep 10, 2020
fa00593
Refactor: Move method, improve comments
chimp1984 Sep 10, 2020
b4d9807
Refactor: Move message classes to new package
chimp1984 Sep 10, 2020
9ee586f
Refactor: Move dispute message classes to new dispute package
chimp1984 Sep 10, 2020
482c73a
Refactor: Rename package
chimp1984 Sep 10, 2020
54e4a08
Only log error at DelayedPayoutTx check if deposit tx is not null
chimp1984 Sep 10, 2020
dce1873
Merge branch 'master_upstream' into add-cancel-trade-feature
chimp1984 Sep 10, 2020
eae6a46
Improve text, dont use action style for button
chimp1984 Sep 10, 2020
4985435
Refactor: Rename classes
chimp1984 Sep 10, 2020
aa6721a
Getting the feature polished... Many small changes...
chimp1984 Sep 10, 2020
5148c77
Mark refreshInterval transient. Move static MAX_REFRESH_INTERVAL to top
chimp1984 Sep 10, 2020
9e917d5
Use more convenient TimeUnit.HOURS.toMillis API
chimp1984 Sep 10, 2020
5c62454
Add comments about wrong usage of trade protocol
chimp1984 Sep 10, 2020
8885e85
Move cancelTrade states to sub classes
chimp1984 Sep 10, 2020
f71b2fb
Update comments, remove todo
chimp1984 Sep 10, 2020
9047ab9
Remove mailbox msg at failure cases in trade protocol tasks
chimp1984 Sep 10, 2020
82fd068
Remove commented out code.
chimp1984 Sep 11, 2020
0f7f3d2
Try to get Codacy to ignore lines
chimp1984 Sep 11, 2020
0e9540d
Remove constructor as Codacy does not like empty constructors.
chimp1984 Sep 11, 2020
6b5e08d
Moved witness signing and msg sending to trade protocol task
chimp1984 Sep 11, 2020
e3c6aa2
Refactoring: Rename param to avoid confusion with allowBroadcast and
chimp1984 Sep 11, 2020
286402f
Add comments
chimp1984 Sep 11, 2020
f819a0d
Let signer broadcast signedWitness as well.
chimp1984 Sep 11, 2020
396c6f4
Use a deterministic id instead of uid
chimp1984 Sep 11, 2020
93f24a4
Refactor: Use SendMailboxMessageTask to avoid duplicated code
chimp1984 Sep 11, 2020
ab2e54f
Resend payment started message as long we dont get an ACK
chimp1984 Sep 11, 2020
873d2cf
Start BuyerSendCounterCurrencyTransferStartedMessage at startup again
chimp1984 Sep 11, 2020
00bb8db
Add comment
chimp1984 Sep 11, 2020
2b05638
Remove dev test value
chimp1984 Sep 11, 2020
bf3ce34
Refactor: Rename signAccountAgeWitness to signAndPublishAccountAgeWit…
chimp1984 Sep 11, 2020
5e46366
Remove duplicate broadcast of witness data
chimp1984 Sep 11, 2020
c2296d4
Send witness with PayoutTxPublishedMessage
chimp1984 Sep 11, 2020
37496c2
Remove refresh button
chimp1984 Sep 11, 2020
d53f5ed
Refactor: Rename display strings
chimp1984 Sep 11, 2020
1742da3
Refactor: Move display strings
chimp1984 Sep 11, 2020
8f7fe51
Merge branch 'master_upstream' into add-cancel-trade-feature
chimp1984 Sep 11, 2020
71e452f
Apply suggestions from code review. Add todos for required changes
chimp1984 Sep 17, 2020
7318b10
Merge branch 'master_upstream' into add-cancel-trade-feature
chimp1984 Sep 17, 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 @@ -272,16 +272,16 @@ private String signAccountAgeWitness(Coin tradeAmount,
return "";
}

public void selfSignAccountAgeWitness(AccountAgeWitness accountAgeWitness) throws CryptoException {
public void signAndPublishAccountAgeWitness(AccountAgeWitness accountAgeWitness) throws CryptoException {
log.info("Sign own accountAgeWitness {}", accountAgeWitness);
signAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness,
signAndPublishAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness,
keyRing.getSignatureKeyPair().getPublic());
}

// Any peer can sign with DSA key
public Optional<SignedWitness> signAccountAgeWitness(Coin tradeAmount,
AccountAgeWitness accountAgeWitness,
PublicKey peersPubKey) throws CryptoException {
public Optional<SignedWitness> signAndPublishAccountAgeWitness(Coin tradeAmount,
AccountAgeWitness accountAgeWitness,
PublicKey peersPubKey) throws CryptoException {
if (isSignedAccountAgeWitness(accountAgeWitness)) {
log.warn("Trader trying to sign already signed accountagewitness {}", accountAgeWitness.toString());
return Optional.empty();
Expand Down Expand Up @@ -494,7 +494,8 @@ void addToMap(SignedWitness signedWitness) {
private void publishSignedWitness(SignedWitness signedWitness) {
if (!signedWitnessMap.containsKey(signedWitness.getHashAsByteArray())) {
log.info("broadcast signed witness {}", signedWitness.toString());
p2PService.addPersistableNetworkPayload(signedWitness, false);
// We set reBroadcast to true to achieve better resilience.
p2PService.addPersistableNetworkPayload(signedWitness, true);
addToMap(signedWitness);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,11 @@
import bisq.core.support.dispute.arbitration.TraderDataItem;
import bisq.core.trade.Contract;
import bisq.core.trade.Trade;
import bisq.core.trade.messages.TraderSignedWitnessMessage;
import bisq.core.trade.protocol.TradingPeer;
import bisq.core.user.User;

import bisq.network.p2p.BootstrapListener;
import bisq.network.p2p.NodeAddress;
import bisq.network.p2p.P2PService;
import bisq.network.p2p.SendMailboxMessageListener;
import bisq.network.p2p.storage.P2PDataStorage;
import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService;

Expand Down Expand Up @@ -76,7 +73,6 @@
import java.util.Optional;
import java.util.Random;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -202,7 +198,7 @@ public void onUpdatedDataReceived() {

private void onBootStrapped() {
republishAllFiatAccounts();
signSameNameAccounts();
signAndPublishSameNameAccounts();
}


Expand Down Expand Up @@ -672,7 +668,7 @@ public void arbitratorSignAccountAgeWitness(AccountAgeWitness accountAgeWitness,
signedWitnessService.signAccountAgeWitness(accountAgeWitness, key, tradersPubKey, time);
}

public Optional<SignedWitness> traderSignPeersAccountAgeWitness(Trade trade) {
public Optional<SignedWitness> traderSignAndPublishPeersAccountAgeWitness(Trade trade) {
AccountAgeWitness peersWitness = findTradePeerWitness(trade).orElse(null);
Coin tradeAmount = trade.getTradeAmount();
checkNotNull(trade.getProcessModel().getTradingPeer().getPubKeyRing(), "Peer must have a keyring");
Expand All @@ -682,7 +678,7 @@ public Optional<SignedWitness> traderSignPeersAccountAgeWitness(Trade trade) {
checkNotNull(peersPubKey, "Peers pub key must not be null");

try {
return signedWitnessService.signAccountAgeWitness(tradeAmount, peersWitness, peersPubKey);
return signedWitnessService.signAndPublishAccountAgeWitness(tradeAmount, peersWitness, peersPubKey);
} catch (CryptoException e) {
log.warn("Trader failed to sign witness, exception {}", e.toString());
}
Expand Down Expand Up @@ -827,7 +823,7 @@ public Set<AccountAgeWitness> getOrphanSignedWitnesses() {
.collect(Collectors.toSet());
}

public void signSameNameAccounts() {
public void signAndPublishSameNameAccounts() {
// Collect accounts that have ownerId to sign unsigned accounts with the same ownderId
var signerAccounts = Objects.requireNonNull(user.getPaymentAccounts()).stream()
.filter(account -> account.getOwnerId() != null &&
Expand All @@ -842,7 +838,7 @@ public void signSameNameAccounts() {
signerAccounts.forEach(signer -> unsignedAccounts.forEach(unsigned -> {
if (signer.getOwnerId().equals(unsigned.getOwnerId())) {
try {
signedWitnessService.selfSignAccountAgeWitness(
signedWitnessService.signAndPublishAccountAgeWitness(
getMyWitness(unsigned.getPaymentAccountPayload()));
} catch (CryptoException e) {
log.warn("Self signing failed, exception {}", e.toString());
Expand All @@ -868,44 +864,4 @@ public boolean isSignWitnessTrade(Trade trade) {
!peerHasSignedWitness(trade) &&
tradeAmountIsSufficient(trade.getTradeAmount());
}

public void maybeSignWitness(Trade trade) {
if (isSignWitnessTrade(trade)) {
var signedWitnessOptional = traderSignPeersAccountAgeWitness(trade);
signedWitnessOptional.ifPresent(signedWitness -> sendSignedWitnessToPeer(signedWitness, trade));
}
}

private void sendSignedWitnessToPeer(SignedWitness signedWitness, Trade trade) {
if (trade == null) return;

NodeAddress tradingPeerNodeAddress = trade.getTradingPeerNodeAddress();
var traderSignedWitnessMessage = new TraderSignedWitnessMessage(UUID.randomUUID().toString(), trade.getId(),
tradingPeerNodeAddress, signedWitness);

p2PService.sendEncryptedMailboxMessage(
tradingPeerNodeAddress,
trade.getProcessModel().getTradingPeer().getPubKeyRing(),
traderSignedWitnessMessage,
new SendMailboxMessageListener() {
@Override
public void onArrived() {
log.info("SendMailboxMessageListener onArrived tradeId={} at peer {} SignedWitness {}",
trade.getId(), tradingPeerNodeAddress, signedWitness);
}

@Override
public void onStoredInMailbox() {
log.info("SendMailboxMessageListener onStoredInMailbox tradeId={} at peer {} SignedWitness {}",
trade.getId(), tradingPeerNodeAddress, signedWitness);
}

@Override
public void onFault(String errorMessage) {
log.error("SendMailboxMessageListener onFault tradeId={} at peer {} SignedWitness {}",
trade.getId(), tradingPeerNodeAddress, signedWitness);
}
}
);
}
}
65 changes: 64 additions & 1 deletion core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

import org.slf4j.Logger;
Expand Down Expand Up @@ -833,6 +832,70 @@ public Transaction sellerSignsAndFinalizesPayoutTx(Transaction depositTx,
return payoutTx;
}

///////////////////////////////////////////////////////////////////////////////////////////
// Canceled trade payoutTx
///////////////////////////////////////////////////////////////////////////////////////////

//TODO avoid code duplication
public byte[] signCanceledTradePayoutTx(Transaction depositTx,
Coin buyerPayoutAmount,
Coin sellerPayoutAmount,
String buyerPayoutAddressString,
String sellerPayoutAddressString,
DeterministicKey myMultiSigKeyPair,
byte[] buyerPubKey,
byte[] sellerPubKey)
throws AddressFormatException, TransactionVerificationException {
Transaction preparedPayoutTx = createPayoutTx(depositTx, buyerPayoutAmount, sellerPayoutAmount,
buyerPayoutAddressString, sellerPayoutAddressString);
// MS redeemScript
Script redeemScript = get2of2MultiSigRedeemScript(buyerPubKey, sellerPubKey);
// MS output from prev. tx is index 0
Sha256Hash sigHash = preparedPayoutTx.hashForSignature(0, redeemScript, Transaction.SigHash.ALL, false);
checkNotNull(myMultiSigKeyPair, "myMultiSigKeyPair must not be null");
if (myMultiSigKeyPair.isEncrypted()) {
checkNotNull(aesKey);
}
ECKey.ECDSASignature mySignature = myMultiSigKeyPair.sign(sigHash, aesKey).toCanonicalised();
WalletService.printTx("prepared canceled trade payoutTx for sig creation", preparedPayoutTx);
WalletService.verifyTransaction(preparedPayoutTx);
return mySignature.encodeToDER();
}

//TODO avoid code duplication
public Transaction finalizeCanceledTradePayoutTx(Transaction depositTx,
chimp1984 marked this conversation as resolved.
Show resolved Hide resolved
byte[] buyerSignature,
byte[] sellerSignature,
Coin buyerPayoutAmount,
Coin sellerPayoutAmount,
String buyerPayoutAddressString,
String sellerPayoutAddressString,
DeterministicKey multiSigKeyPair,
byte[] buyerPubKey,
byte[] sellerPubKey)
throws AddressFormatException, TransactionVerificationException, WalletException {
Transaction payoutTx = createPayoutTx(depositTx, buyerPayoutAmount, sellerPayoutAmount, buyerPayoutAddressString, sellerPayoutAddressString);
// MS redeemScript
Script redeemScript = get2of2MultiSigRedeemScript(buyerPubKey, sellerPubKey);
// MS output from prev. tx is index 0
checkNotNull(multiSigKeyPair, "multiSigKeyPair must not be null");
TransactionSignature buyerTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(buyerSignature),
Transaction.SigHash.ALL, false);
TransactionSignature sellerTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(sellerSignature),
Transaction.SigHash.ALL, false);
// Take care of order of signatures. Need to be reversed here. See comment below at getMultiSigRedeemScript (seller, buyer)
Script inputScript = ScriptBuilder.createP2SHMultiSigInputScript(ImmutableList.of(sellerTxSig, buyerTxSig), redeemScript);
TransactionInput input = payoutTx.getInput(0);
input.setScriptSig(inputScript);
WalletService.printTx("canceled trade payoutTx", payoutTx);
WalletService.verifyTransaction(payoutTx);
WalletService.checkWalletConsistency(wallet);
WalletService.checkScriptSig(payoutTx, input, 0);
checkNotNull(input.getConnectedOutput(), "input.getConnectedOutput() must not be null");
input.verify(input.getConnectedOutput());
return payoutTx;
}


///////////////////////////////////////////////////////////////////////////////////////////
// Mediated payoutTx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,15 @@
import bisq.core.trade.messages.DepositTxMessage;
import bisq.core.trade.messages.InputsForDepositTxRequest;
import bisq.core.trade.messages.InputsForDepositTxResponse;
import bisq.core.trade.messages.MediatedPayoutTxPublishedMessage;
import bisq.core.trade.messages.MediatedPayoutTxSignatureMessage;
import bisq.core.trade.messages.PayoutTxPublishedMessage;
import bisq.core.trade.messages.PeerPublishedDelayedPayoutTxMessage;
import bisq.core.trade.messages.RefreshTradeStateRequest;
import bisq.core.trade.messages.TraderSignedWitnessMessage;
import bisq.core.trade.messages.cancel.CancelTradeRequestAcceptedMessage;
import bisq.core.trade.messages.cancel.CancelTradeRequestRejectedMessage;
import bisq.core.trade.messages.cancel.RequestCancelTradeMessage;
import bisq.core.trade.messages.mediation.MediatedPayoutTxPublishedMessage;
import bisq.core.trade.messages.mediation.MediatedPayoutTxSignatureMessage;
import bisq.core.trade.statistics.TradeStatistics;

import bisq.network.p2p.AckMessage;
Expand Down Expand Up @@ -174,6 +177,13 @@ public NetworkEnvelope fromProto(protobuf.NetworkEnvelope proto) throws Protobuf
case MEDIATED_PAYOUT_TX_PUBLISHED_MESSAGE:
return MediatedPayoutTxPublishedMessage.fromProto(proto.getMediatedPayoutTxPublishedMessage(), messageVersion);

case REQUEST_CANCEL_TRADE_MESSAGE:
return RequestCancelTradeMessage.fromProto(proto.getRequestCancelTradeMessage(), messageVersion);
case CANCEL_TRADE_ACCEPTED_MESSAGE:
return CancelTradeRequestAcceptedMessage.fromProto(proto.getCancelTradeAcceptedMessage(), messageVersion);
case CANCEL_TRADE_REJECTED_MESSAGE:
return CancelTradeRequestRejectedMessage.fromProto(proto.getCancelTradeRejectedMessage(), messageVersion);

case OPEN_NEW_DISPUTE_MESSAGE:
return OpenNewDisputeMessage.fromProto(proto.getOpenNewDisputeMessage(), this, messageVersion);
case PEER_OPENED_DISPUTE_MESSAGE:
Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/bisq/core/trade/BuyerAsMakerTrade.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public static Tradable fromProto(protobuf.BuyerAsMakerTrade buyerAsMakerTradePro
trade.setTradePrice(proto.getTradePrice());
trade.setTradingPeerNodeAddress(proto.hasTradingPeerNodeAddress() ? NodeAddress.fromProto(proto.getTradingPeerNodeAddress()) : null);

return fromProto(trade,
return BuyerTrade.fromProto(trade,
proto,
coreProtoResolver);
}
Expand All @@ -115,4 +115,10 @@ public void handleTakeOfferRequest(InputsForDepositTxRequest message,
ErrorMessageHandler errorMessageHandler) {
((MakerProtocol) tradeProtocol).handleTakeOfferRequest(message, taker, errorMessageHandler);
}


@Override
public String toString() {
return "BuyerAsMakerTrade{} " + super.toString();
}
}
8 changes: 7 additions & 1 deletion core/src/main/java/bisq/core/trade/BuyerAsTakerTrade.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static Tradable fromProto(protobuf.BuyerAsTakerTrade buyerAsTakerTradePro
BtcWalletService btcWalletService,
CoreProtoResolver coreProtoResolver) {
protobuf.Trade proto = buyerAsTakerTradeProto.getTrade();
return fromProto(new BuyerAsTakerTrade(
return BuyerTrade.fromProto(new BuyerAsTakerTrade(
Offer.fromProto(proto.getOffer()),
Coin.valueOf(proto.getTradeAmountAsLong()),
Coin.valueOf(proto.getTxFeeAsLong()),
Expand Down Expand Up @@ -118,4 +118,10 @@ public void takeAvailableOffer() {
checkArgument(tradeProtocol instanceof TakerProtocol, "tradeProtocol NOT instanceof TakerProtocol");
((TakerProtocol) tradeProtocol).takeAvailableOffer();
}


@Override
public String toString() {
return "BuyerAsTakerTrade{} " + super.toString();
}
}
Loading