From 527f1537a9e9525fcdc92b2931e5bd64f8965669 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 22 Sep 2020 00:32:28 -0500 Subject: [PATCH] Add listener on BuyerSendCounterCurrencyTransferStartedMessage to resend msg case it has not arrived Add signed witness to PayoutTxPublishedMessage Remove usage of RefreshTradeStateRequest but leave it for backward compatibility Move removeMailboxMessageAfterProcessing calls in finally branch Rename methods --- .../account/sign/SignedWitnessService.java | 45 +++--- .../witness/AccountAgeWitnessService.java | 60 +------ .../java/bisq/core/trade/TradeManager.java | 3 - .../messages/PayoutTxPublishedMessage.java | 43 +++-- .../messages/RefreshTradeStateRequest.java | 24 +-- .../trade/protocol/BuyerAsMakerProtocol.java | 35 ++-- .../trade/protocol/BuyerAsTakerProtocol.java | 35 ++-- .../core/trade/protocol/TradeProtocol.java | 14 -- ...ssPeerPublishedDelayedPayoutTxMessage.java | 3 +- .../tasks/SendMailboxMessageTask.java | 92 +++++++++++ .../tasks/SendPayoutTxPublishedMessage.java | 97 ----------- ...essDepositTxAndDelayedPayoutTxMessage.java | 4 +- .../BuyerProcessPayoutTxPublishedMessage.java | 13 +- ...CounterCurrencyTransferStartedMessage.java | 151 +++++++++++------- ...ProcessMediatedPayoutSignatureMessage.java | 3 +- ...ocessMediatedPayoutTxPublishedMessage.java | 3 +- .../SendMediatedPayoutTxPublishedMessage.java | 10 +- ...CounterCurrencyTransferStartedMessage.java | 4 +- .../SellerSendPayoutTxPublishedMessage.java | 35 +++- .../sign/SignedWitnessServiceTest.java | 4 +- .../witness/AccountAgeWitnessServiceTest.java | 2 +- .../fiataccounts/FiatAccountsDataModel.java | 2 +- .../pendingtrades/PendingTradesDataModel.java | 41 ----- .../pendingtrades/PendingTradesView.java | 2 - .../steps/seller/SellerStep2View.java | 60 ------- proto/src/main/proto/pb.proto | 19 ++- 26 files changed, 358 insertions(+), 446 deletions(-) create mode 100644 core/src/main/java/bisq/core/trade/protocol/tasks/SendMailboxMessageTask.java delete mode 100644 core/src/main/java/bisq/core/trade/protocol/tasks/SendPayoutTxPublishedMessage.java diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java index e27bf2493c..788c369eed 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java @@ -216,20 +216,20 @@ public class SignedWitnessService { } // Arbitrators sign with EC key - public void signAccountAgeWitness(Coin tradeAmount, - AccountAgeWitness accountAgeWitness, - ECKey key, - PublicKey peersPubKey) { - signAccountAgeWitness(tradeAmount, accountAgeWitness, key, peersPubKey.getEncoded(), new Date().getTime()); + public void signAndPublishAccountAgeWitness(Coin tradeAmount, + AccountAgeWitness accountAgeWitness, + ECKey key, + PublicKey peersPubKey) { + signAndPublishAccountAgeWitness(tradeAmount, accountAgeWitness, key, peersPubKey.getEncoded(), new Date().getTime()); } // Arbitrators sign with EC key - public String signAccountAgeWitness(AccountAgeWitness accountAgeWitness, - ECKey key, - byte[] peersPubKey, - long time) { + public String signAndPublishAccountAgeWitness(AccountAgeWitness accountAgeWitness, + ECKey key, + byte[] peersPubKey, + long time) { var witnessPubKey = peersPubKey == null ? ownerPubKey(accountAgeWitness) : peersPubKey; - return signAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness, key, witnessPubKey, time); + return signAndPublishAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness, key, witnessPubKey, time); } // Arbitrators sign with EC key @@ -238,15 +238,15 @@ public class SignedWitnessService { long childSignTime) { var time = childSignTime - SIGNER_AGE - 1; var dummyAccountAgeWitness = new AccountAgeWitness(Hash.getRipemd160hash(peersPubKey), time); - return signAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, dummyAccountAgeWitness, key, peersPubKey, time); + return signAndPublishAccountAgeWitness(MINIMUM_TRADE_AMOUNT_FOR_SIGNING, dummyAccountAgeWitness, key, peersPubKey, time); } // Arbitrators sign with EC key - private String signAccountAgeWitness(Coin tradeAmount, - AccountAgeWitness accountAgeWitness, - ECKey key, - byte[] peersPubKey, - long time) { + private String signAndPublishAccountAgeWitness(Coin tradeAmount, + AccountAgeWitness accountAgeWitness, + ECKey key, + byte[] peersPubKey, + long time) { if (isSignedAccountAgeWitness(accountAgeWitness)) { var err = "Arbitrator trying to sign already signed accountagewitness " + accountAgeWitness.toString(); log.warn(err); @@ -272,16 +272,16 @@ public class SignedWitnessService { return ""; } - public void selfSignAccountAgeWitness(AccountAgeWitness accountAgeWitness) throws CryptoException { + public void selfSignAndPublishAccountAgeWitness(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 signAccountAgeWitness(Coin tradeAmount, - AccountAgeWitness accountAgeWitness, - PublicKey peersPubKey) throws CryptoException { + public Optional 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(); @@ -494,7 +494,8 @@ public class SignedWitnessService { 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); } } diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java index c5264fc4ed..ab8ad18e17 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java @@ -35,14 +35,11 @@ import bisq.core.support.dispute.DisputeResult; 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; @@ -76,7 +73,6 @@ import java.util.Objects; 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; @@ -202,7 +198,7 @@ public class AccountAgeWitnessService { private void onBootStrapped() { republishAllFiatAccounts(); - signSameNameAccounts(); + signAndPublishSameNameAccounts(); } @@ -643,7 +639,7 @@ public class AccountAgeWitnessService { AccountAgeWitness accountAgeWitness, ECKey key, PublicKey peersPubKey) { - signedWitnessService.signAccountAgeWitness(tradeAmount, accountAgeWitness, key, peersPubKey); + signedWitnessService.signAndPublishAccountAgeWitness(tradeAmount, accountAgeWitness, key, peersPubKey); } public String arbitratorSignOrphanWitness(AccountAgeWitness accountAgeWitness, @@ -655,7 +651,7 @@ public class AccountAgeWitnessService { .findAny() .orElse(null); checkNotNull(signedWitness); - return signedWitnessService.signAccountAgeWitness(accountAgeWitness, key, signedWitness.getWitnessOwnerPubKey(), + return signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, key, signedWitness.getWitnessOwnerPubKey(), time); } @@ -669,10 +665,10 @@ public class AccountAgeWitnessService { ECKey key, byte[] tradersPubKey, long time) { - signedWitnessService.signAccountAgeWitness(accountAgeWitness, key, tradersPubKey, time); + signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, key, tradersPubKey, time); } - public Optional traderSignPeersAccountAgeWitness(Trade trade) { + public Optional traderSignAndPublishPeersAccountAgeWitness(Trade trade) { AccountAgeWitness peersWitness = findTradePeerWitness(trade).orElse(null); Coin tradeAmount = trade.getTradeAmount(); checkNotNull(trade.getProcessModel().getTradingPeer().getPubKeyRing(), "Peer must have a keyring"); @@ -682,7 +678,7 @@ public class AccountAgeWitnessService { 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()); } @@ -827,7 +823,7 @@ public class AccountAgeWitnessService { .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 && @@ -842,7 +838,7 @@ public class AccountAgeWitnessService { signerAccounts.forEach(signer -> unsignedAccounts.forEach(unsigned -> { if (signer.getOwnerId().equals(unsigned.getOwnerId())) { try { - signedWitnessService.selfSignAccountAgeWitness( + signedWitnessService.selfSignAndPublishAccountAgeWitness( getMyWitness(unsigned.getPaymentAccountPayload())); } catch (CryptoException e) { log.warn("Self signing failed, exception {}", e.toString()); @@ -868,44 +864,4 @@ public class AccountAgeWitnessService { !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); - } - } - ); - } } diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index 817dcfe0b8..018b1200ac 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -350,9 +350,6 @@ public class TradeManager implements PersistedDataHost { ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { sellerTrade.onFiatPaymentReceived(resultHandler, errorMessageHandler); - - //TODO move to trade protocol task - accountAgeWitnessService.maybeSignWitness(sellerTrade); } private void initPendingTrade(Trade trade) { diff --git a/core/src/main/java/bisq/core/trade/messages/PayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/messages/PayoutTxPublishedMessage.java index cae6469c1e..6b6f38f9c1 100644 --- a/core/src/main/java/bisq/core/trade/messages/PayoutTxPublishedMessage.java +++ b/core/src/main/java/bisq/core/trade/messages/PayoutTxPublishedMessage.java @@ -17,6 +17,8 @@ package bisq.core.trade.messages; +import bisq.core.account.sign.SignedWitness; + import bisq.network.p2p.MailboxMessage; import bisq.network.p2p.NodeAddress; @@ -26,23 +28,35 @@ import bisq.common.util.Utilities; import com.google.protobuf.ByteString; +import java.util.Optional; +import java.util.UUID; + import lombok.EqualsAndHashCode; import lombok.Value; +import lombok.extern.slf4j.Slf4j; +import javax.annotation.Nullable; + +@Slf4j @EqualsAndHashCode(callSuper = true) @Value public final class PayoutTxPublishedMessage extends TradeMessage implements MailboxMessage { private final byte[] payoutTx; private final NodeAddress senderNodeAddress; + // Added in v1.4.0 + @Nullable + private final SignedWitness signedWitness; + public PayoutTxPublishedMessage(String tradeId, byte[] payoutTx, NodeAddress senderNodeAddress, - String uid) { + @Nullable SignedWitness signedWitness) { this(tradeId, payoutTx, senderNodeAddress, - uid, + signedWitness, + UUID.randomUUID().toString(), Version.getP2PMessageVersion()); } @@ -54,28 +68,37 @@ public final class PayoutTxPublishedMessage extends TradeMessage implements Mail private PayoutTxPublishedMessage(String tradeId, byte[] payoutTx, NodeAddress senderNodeAddress, + @Nullable SignedWitness signedWitness, String uid, int messageVersion) { super(messageVersion, tradeId, uid); this.payoutTx = payoutTx; this.senderNodeAddress = senderNodeAddress; + this.signedWitness = signedWitness; } @Override public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { - return getNetworkEnvelopeBuilder() - .setPayoutTxPublishedMessage(protobuf.PayoutTxPublishedMessage.newBuilder() - .setTradeId(tradeId) - .setPayoutTx(ByteString.copyFrom(payoutTx)) - .setSenderNodeAddress(senderNodeAddress.toProtoMessage()) - .setUid(uid)) - .build(); + protobuf.PayoutTxPublishedMessage.Builder builder = protobuf.PayoutTxPublishedMessage.newBuilder() + .setTradeId(tradeId) + .setPayoutTx(ByteString.copyFrom(payoutTx)) + .setSenderNodeAddress(senderNodeAddress.toProtoMessage()) + .setUid(uid); + Optional.ofNullable(signedWitness).ifPresent(signedWitness -> builder.setSignedWitness(signedWitness.toProtoSignedWitness())); + return getNetworkEnvelopeBuilder().setPayoutTxPublishedMessage(builder).build(); } public static NetworkEnvelope fromProto(protobuf.PayoutTxPublishedMessage proto, int messageVersion) { + // There is no method to check for a nullable non-primitive data type object but we know that all fields + // are empty/null, so we check for the signature to see if we got a valid signedWitness. + protobuf.SignedWitness protoSignedWitness = proto.getSignedWitness(); + SignedWitness signedWitness = !protoSignedWitness.getSignature().isEmpty() ? + SignedWitness.fromProto(protoSignedWitness) : + null; return new PayoutTxPublishedMessage(proto.getTradeId(), proto.getPayoutTx().toByteArray(), NodeAddress.fromProto(proto.getSenderNodeAddress()), + signedWitness, proto.getUid(), messageVersion); } @@ -85,7 +108,7 @@ public final class PayoutTxPublishedMessage extends TradeMessage implements Mail return "PayoutTxPublishedMessage{" + "\n payoutTx=" + Utilities.bytesAsHexString(payoutTx) + ",\n senderNodeAddress=" + senderNodeAddress + - ",\n uid='" + uid + '\'' + + ",\n signedWitness=" + signedWitness + "\n} " + super.toString(); } } diff --git a/core/src/main/java/bisq/core/trade/messages/RefreshTradeStateRequest.java b/core/src/main/java/bisq/core/trade/messages/RefreshTradeStateRequest.java index 907af09a65..ff5c82362e 100644 --- a/core/src/main/java/bisq/core/trade/messages/RefreshTradeStateRequest.java +++ b/core/src/main/java/bisq/core/trade/messages/RefreshTradeStateRequest.java @@ -20,25 +20,19 @@ package bisq.core.trade.messages; import bisq.network.p2p.MailboxMessage; import bisq.network.p2p.NodeAddress; -import bisq.common.app.Version; - import lombok.EqualsAndHashCode; import lombok.Value; +/** + * Not used anymore since v1.4.0 + * We do the re-sending of the payment sent message via the BuyerSendCounterCurrencyTransferStartedMessage task on the + * buyer side, so seller do not need to do anything interactively. + */ @EqualsAndHashCode(callSuper = true) @Value public class RefreshTradeStateRequest extends TradeMessage implements MailboxMessage { private final NodeAddress senderNodeAddress; - public RefreshTradeStateRequest(String uid, - String tradeId, - NodeAddress senderNodeAddress) { - this(Version.getP2PMessageVersion(), - uid, - tradeId, - senderNodeAddress); - } - /////////////////////////////////////////////////////////////////////////////////////////// // PROTO BUFFER @@ -67,12 +61,4 @@ public class RefreshTradeStateRequest extends TradeMessage implements MailboxMes proto.getTradeId(), NodeAddress.fromProto(proto.getSenderNodeAddress())); } - - @Override - public String toString() { - return "RefreshTradeStateRequest{" + - "\n senderNodeAddress=" + senderNodeAddress + - "\n} " + super.toString(); - } - } diff --git a/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java index 6803fe2d97..515572eff3 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java @@ -23,7 +23,6 @@ import bisq.core.trade.messages.DelayedPayoutTxSignatureRequest; import bisq.core.trade.messages.DepositTxAndDelayedPayoutTxMessage; import bisq.core.trade.messages.InputsForDepositTxRequest; import bisq.core.trade.messages.PayoutTxPublishedMessage; -import bisq.core.trade.messages.RefreshTradeStateRequest; import bisq.core.trade.messages.TradeMessage; import bisq.core.trade.protocol.tasks.ApplyFilter; import bisq.core.trade.protocol.tasks.PublishTradeStatistics; @@ -87,6 +86,19 @@ public class BuyerAsMakerProtocol extends TradeProtocol implements BuyerProtocol taskRunner.addTasks(BuyerSetupPayoutTxListener.class); taskRunner.run(); } + + // We might have 2 taskRunners as BuyerSetupPayoutTxListener might have been started as well + if (trade.getState() == Trade.State.BUYER_STORED_IN_MAILBOX_FIAT_PAYMENT_INITIATED_MSG || + trade.getState() == Trade.State.BUYER_SEND_FAILED_FIAT_PAYMENT_INITIATED_MSG) { + // In case we have not received an ACK from the CounterCurrencyTransferStartedMessage we re-send it + // periodically in BuyerSendCounterCurrencyTransferStartedMessage + TradeTaskRunner taskRunner = new TradeTaskRunner(trade, + () -> handleTaskRunnerSuccess("BuyerSendCounterCurrencyTransferStartedMessage"), + this::handleTaskRunnerFault); + + taskRunner.addTasks(BuyerSendCounterCurrencyTransferStartedMessage.class); + taskRunner.run(); + } } @@ -102,8 +114,6 @@ public class BuyerAsMakerProtocol extends TradeProtocol implements BuyerProtocol handle((DepositTxAndDelayedPayoutTxMessage) tradeMessage, peerNodeAddress); } else if (tradeMessage instanceof PayoutTxPublishedMessage) { handle((PayoutTxPublishedMessage) tradeMessage, peerNodeAddress); - } else if (tradeMessage instanceof RefreshTradeStateRequest) { - handle(); } } @@ -234,23 +244,6 @@ public class BuyerAsMakerProtocol extends TradeProtocol implements BuyerProtocol } - /////////////////////////////////////////////////////////////////////////////////////////// - // Incoming message Handle missing messages - /////////////////////////////////////////////////////////////////////////////////////////// - - private void handle() { - // Resend CounterCurrencyTransferStartedMessage if it hasn't been acked yet and counterparty asked for a refresh - if (trade.getState().getPhase() == Trade.Phase.FIAT_SENT && - trade.getState().ordinal() >= Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG.ordinal()) { - TradeTaskRunner taskRunner = new TradeTaskRunner(buyerAsMakerTrade, - () -> handleTaskRunnerSuccess("RefreshTradeStateRequest"), - this::handleTaskRunnerFault); - taskRunner.addTasks(BuyerSendCounterCurrencyTransferStartedMessage.class); - taskRunner.run(); - } - } - - /////////////////////////////////////////////////////////////////////////////////////////// // Message dispatcher /////////////////////////////////////////////////////////////////////////////////////////// @@ -268,8 +261,6 @@ public class BuyerAsMakerProtocol extends TradeProtocol implements BuyerProtocol handle((DepositTxAndDelayedPayoutTxMessage) tradeMessage, sender); } else if (tradeMessage instanceof PayoutTxPublishedMessage) { handle((PayoutTxPublishedMessage) tradeMessage, sender); - } else if (tradeMessage instanceof RefreshTradeStateRequest) { - handle(); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java index 9698953624..4073167d13 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java @@ -25,7 +25,6 @@ import bisq.core.trade.messages.DelayedPayoutTxSignatureRequest; import bisq.core.trade.messages.DepositTxAndDelayedPayoutTxMessage; import bisq.core.trade.messages.InputsForDepositTxResponse; import bisq.core.trade.messages.PayoutTxPublishedMessage; -import bisq.core.trade.messages.RefreshTradeStateRequest; import bisq.core.trade.messages.TradeMessage; import bisq.core.trade.protocol.tasks.ApplyFilter; import bisq.core.trade.protocol.tasks.PublishTradeStatistics; @@ -95,6 +94,19 @@ public class BuyerAsTakerProtocol extends TradeProtocol implements BuyerProtocol taskRunner.addTasks(BuyerSetupPayoutTxListener.class); taskRunner.run(); } + + // We might have 2 taskRunners as BuyerSetupPayoutTxListener might have been started as well + if (trade.getState() == Trade.State.BUYER_STORED_IN_MAILBOX_FIAT_PAYMENT_INITIATED_MSG || + trade.getState() == Trade.State.BUYER_SEND_FAILED_FIAT_PAYMENT_INITIATED_MSG) { + // In case we have not received an ACK from the CounterCurrencyTransferStartedMessage we re-send it + // periodically in BuyerSendCounterCurrencyTransferStartedMessage + TradeTaskRunner taskRunner = new TradeTaskRunner(trade, + () -> handleTaskRunnerSuccess("BuyerSendCounterCurrencyTransferStartedMessage"), + this::handleTaskRunnerFault); + + taskRunner.addTasks(BuyerSendCounterCurrencyTransferStartedMessage.class); + taskRunner.run(); + } } @@ -110,8 +122,6 @@ public class BuyerAsTakerProtocol extends TradeProtocol implements BuyerProtocol handle((DepositTxAndDelayedPayoutTxMessage) tradeMessage, peerNodeAddress); } else if (tradeMessage instanceof PayoutTxPublishedMessage) { handle((PayoutTxPublishedMessage) tradeMessage, peerNodeAddress); - } else if (tradeMessage instanceof RefreshTradeStateRequest) { - handle(); } } @@ -264,23 +274,6 @@ public class BuyerAsTakerProtocol extends TradeProtocol implements BuyerProtocol } - /////////////////////////////////////////////////////////////////////////////////////////// - // Incoming message Handle missing messages - /////////////////////////////////////////////////////////////////////////////////////////// - - private void handle() { - // Resend CounterCurrencyTransferStartedMessage if it hasn't been acked yet and counterparty asked for a refresh - if (trade.getState().getPhase() == Trade.Phase.FIAT_SENT && - trade.getState().ordinal() >= Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG.ordinal()) { - TradeTaskRunner taskRunner = new TradeTaskRunner(buyerAsTakerTrade, - () -> handleTaskRunnerSuccess("RefreshTradeStateRequest"), - this::handleTaskRunnerFault); - taskRunner.addTasks(BuyerSendCounterCurrencyTransferStartedMessage.class); - taskRunner.run(); - } - } - - /////////////////////////////////////////////////////////////////////////////////////////// // Message dispatcher /////////////////////////////////////////////////////////////////////////////////////////// @@ -300,8 +293,6 @@ public class BuyerAsTakerProtocol extends TradeProtocol implements BuyerProtocol handle((DepositTxAndDelayedPayoutTxMessage) tradeMessage, sender); } else if (tradeMessage instanceof PayoutTxPublishedMessage) { handle((PayoutTxPublishedMessage) tradeMessage, sender); - } else if (tradeMessage instanceof RefreshTradeStateRequest) { - handle(); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/TradeProtocol.java b/core/src/main/java/bisq/core/trade/protocol/TradeProtocol.java index be41e046f8..aafb8303d9 100644 --- a/core/src/main/java/bisq/core/trade/protocol/TradeProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/TradeProtocol.java @@ -25,7 +25,6 @@ import bisq.core.trade.messages.MediatedPayoutTxPublishedMessage; import bisq.core.trade.messages.MediatedPayoutTxSignatureMessage; import bisq.core.trade.messages.PeerPublishedDelayedPayoutTxMessage; import bisq.core.trade.messages.TradeMessage; -import bisq.core.trade.messages.TraderSignedWitnessMessage; import bisq.core.trade.protocol.tasks.ApplyFilter; import bisq.core.trade.protocol.tasks.ProcessPeerPublishedDelayedPayoutTxMessage; import bisq.core.trade.protocol.tasks.mediation.BroadcastMediatedPayoutTx; @@ -227,15 +226,6 @@ public abstract class TradeProtocol { taskRunner.run(); } - /////////////////////////////////////////////////////////////////////////////////////////// - // Peer has sent a SignedWitness - /////////////////////////////////////////////////////////////////////////////////////////// - - private void handle(TraderSignedWitnessMessage tradeMessage) { - // Publish signed witness, if it is valid and ours - processModel.getAccountAgeWitnessService().publishOwnSignedWitness(tradeMessage.getSignedWitness()); - } - /////////////////////////////////////////////////////////////////////////////////////////// // Dispatcher @@ -248,8 +238,6 @@ public abstract class TradeProtocol { handle((MediatedPayoutTxPublishedMessage) tradeMessage, sender); } else if (tradeMessage instanceof PeerPublishedDelayedPayoutTxMessage) { handle((PeerPublishedDelayedPayoutTxMessage) tradeMessage, sender); - } else if (tradeMessage instanceof TraderSignedWitnessMessage) { - handle((TraderSignedWitnessMessage) tradeMessage); } } @@ -297,8 +285,6 @@ public abstract class TradeProtocol { handle((MediatedPayoutTxPublishedMessage) tradeMessage, peerNodeAddress); } else if (tradeMessage instanceof PeerPublishedDelayedPayoutTxMessage) { handle((PeerPublishedDelayedPayoutTxMessage) tradeMessage, peerNodeAddress); - } else if (tradeMessage instanceof TraderSignedWitnessMessage) { - handle((TraderSignedWitnessMessage) tradeMessage); } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessPeerPublishedDelayedPayoutTxMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessPeerPublishedDelayedPayoutTxMessage.java index c4aae4dd38..385e863850 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessPeerPublishedDelayedPayoutTxMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessPeerPublishedDelayedPayoutTxMessage.java @@ -47,7 +47,6 @@ public class ProcessPeerPublishedDelayedPayoutTxMessage extends TradeTask { // update to the latest peer address of our peer if the message is correct trade.setTradingPeerNodeAddress(processModel.getTempTradingPeerNodeAddress()); - processModel.removeMailboxMessageAfterProcessing(trade); // We add the tx to our wallet. Transaction delayedPayoutTx = checkNotNull(trade.getDelayedPayoutTx()); @@ -58,6 +57,8 @@ public class ProcessPeerPublishedDelayedPayoutTxMessage extends TradeTask { complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/SendMailboxMessageTask.java b/core/src/main/java/bisq/core/trade/protocol/tasks/SendMailboxMessageTask.java new file mode 100644 index 0000000000..29aeb0d5cf --- /dev/null +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/SendMailboxMessageTask.java @@ -0,0 +1,92 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.trade.protocol.tasks; + +import bisq.core.trade.Trade; +import bisq.core.trade.messages.TradeMessage; + +import bisq.network.p2p.NodeAddress; +import bisq.network.p2p.SendMailboxMessageListener; + +import bisq.common.taskrunner.TaskRunner; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public abstract class SendMailboxMessageTask extends TradeTask { + public SendMailboxMessageTask(TaskRunner taskHandler, Trade trade) { + super(taskHandler, trade); + } + + protected abstract TradeMessage getMessage(String id); + + protected abstract void setStateSent(); + + protected abstract void setStateArrived(); + + protected abstract void setStateStoredInMailbox(); + + protected abstract void setStateFault(); + + @Override + protected void run() { + try { + runInterceptHook(); + String id = processModel.getOfferId(); + TradeMessage message = getMessage(id); + setStateSent(); + NodeAddress peersNodeAddress = trade.getTradingPeerNodeAddress(); + log.info("Send {} to peer {}. tradeId={}, uid={}", + message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); + + processModel.getP2PService().sendEncryptedMailboxMessage( + peersNodeAddress, + processModel.getTradingPeer().getPubKeyRing(), + message, + new SendMailboxMessageListener() { + @Override + public void onArrived() { + log.info("{} arrived at peer {}. tradeId={}, uid={}", + message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); + setStateArrived(); + complete(); + } + + @Override + public void onStoredInMailbox() { + log.info("{} stored in mailbox for peer {}. tradeId={}, uid={}", + message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); + setStateStoredInMailbox(); + complete(); + } + + @Override + public void onFault(String errorMessage) { + log.error("{} failed: Peer {}. tradeId={}, uid={}, errorMessage={}", + message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid(), errorMessage); + setStateFault(); + appendToErrorMessage("Sending message failed: message=" + message + "\nerrorMessage=" + errorMessage); + failed(errorMessage); + } + } + ); + } catch (Throwable t) { + failed(t); + } + } +} diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/SendPayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/SendPayoutTxPublishedMessage.java deleted file mode 100644 index ea85430c9a..0000000000 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/SendPayoutTxPublishedMessage.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * This file is part of Bisq. - * - * Bisq is free software: you can redistribute it and/or modify it - * under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or (at - * your option) any later version. - * - * Bisq is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public - * License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with Bisq. If not, see . - */ - -package bisq.core.trade.protocol.tasks; - -import bisq.core.trade.Trade; -import bisq.core.trade.messages.TradeMessage; - -import bisq.network.p2p.NodeAddress; -import bisq.network.p2p.SendMailboxMessageListener; - -import bisq.common.taskrunner.TaskRunner; - -import lombok.extern.slf4j.Slf4j; - -@Slf4j -public abstract class SendPayoutTxPublishedMessage extends TradeTask { - public SendPayoutTxPublishedMessage(TaskRunner taskHandler, Trade trade) { - super(taskHandler, trade); - } - - protected abstract TradeMessage getMessage(String id); - - protected abstract void setStateSent(); - - protected abstract void setStateArrived(); - - protected abstract void setStateStoredInMailbox(); - - protected abstract void setStateFault(); - - @Override - protected void run() { - try { - runInterceptHook(); - if (trade.getPayoutTx() != null) { - String id = processModel.getOfferId(); - TradeMessage message = getMessage(id); - setStateSent(); - NodeAddress peersNodeAddress = trade.getTradingPeerNodeAddress(); - log.info("Send {} to peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - - processModel.getP2PService().sendEncryptedMailboxMessage( - peersNodeAddress, - processModel.getTradingPeer().getPubKeyRing(), - message, - new SendMailboxMessageListener() { - @Override - public void onArrived() { - log.info("{} arrived at peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - setStateArrived(); - complete(); - } - - @Override - public void onStoredInMailbox() { - log.info("{} stored in mailbox for peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - setStateStoredInMailbox(); - complete(); - } - - @Override - public void onFault(String errorMessage) { - log.error("{} failed: Peer {}. tradeId={}, uid={}, errorMessage={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid(), errorMessage); - setStateFault(); - appendToErrorMessage("Sending message failed: message=" + message + "\nerrorMessage=" + errorMessage); - failed(errorMessage); - } - } - ); - } else { - log.error("trade.getPayoutTx() = " + trade.getPayoutTx()); - failed("PayoutTx is null"); - } - } catch (Throwable t) { - failed(t); - } - } -} diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDepositTxAndDelayedPayoutTxMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDepositTxAndDelayedPayoutTxMessage.java index 60ac804801..09683d9d99 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDepositTxAndDelayedPayoutTxMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDepositTxAndDelayedPayoutTxMessage.java @@ -65,8 +65,6 @@ public class BuyerProcessDepositTxAndDelayedPayoutTxMessage extends TradeTask { trade.setTradingPeerNodeAddress(processModel.getTempTradingPeerNodeAddress()); - processModel.removeMailboxMessageAfterProcessing(trade); - // If we got already the confirmation we don't want to apply an earlier state if (trade.getState().ordinal() < Trade.State.BUYER_SAW_DEPOSIT_TX_IN_NETWORK.ordinal()) { trade.setState(Trade.State.BUYER_RECEIVED_DEPOSIT_TX_PUBLISHED_MSG); @@ -78,6 +76,8 @@ public class BuyerProcessDepositTxAndDelayedPayoutTxMessage extends TradeTask { complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessPayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessPayoutTxPublishedMessage.java index ebcf85eaa2..f82bb916be 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessPayoutTxPublishedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessPayoutTxPublishedMessage.java @@ -17,6 +17,7 @@ package bisq.core.trade.protocol.tasks.buyer; +import bisq.core.account.sign.SignedWitness; import bisq.core.btc.model.AddressEntry; import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletService; @@ -63,10 +64,20 @@ public class BuyerProcessPayoutTxPublishedMessage extends TradeTask { } else { log.info("We got the payout tx already set from BuyerSetupPayoutTxListener and do nothing here. trade ID={}", trade.getId()); } - processModel.removeMailboxMessageAfterProcessing(trade); + + SignedWitness signedWitness = message.getSignedWitness(); + if (signedWitness != null) { + // We received the signedWitness from the seller and publish the data to the network. + // The signer has published it as well but we prefer to re-do it on our side as well to achieve higher + // resilience. + processModel.getAccountAgeWitnessService().publishOwnSignedWitness(signedWitness); + } + complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSendCounterCurrencyTransferStartedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSendCounterCurrencyTransferStartedMessage.java index 2804eb25ed..922e0829b5 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSendCounterCurrencyTransferStartedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSendCounterCurrencyTransferStartedMessage.java @@ -18,81 +18,122 @@ package bisq.core.trade.protocol.tasks.buyer; import bisq.core.btc.model.AddressEntry; -import bisq.core.btc.wallet.BtcWalletService; +import bisq.core.network.MessageState; +import bisq.core.payment.payload.PaymentMethod; import bisq.core.trade.Trade; import bisq.core.trade.messages.CounterCurrencyTransferStartedMessage; -import bisq.core.trade.protocol.tasks.TradeTask; - -import bisq.network.p2p.NodeAddress; -import bisq.network.p2p.SendMailboxMessageListener; +import bisq.core.trade.messages.TradeMessage; +import bisq.core.trade.protocol.tasks.SendMailboxMessageTask; +import bisq.common.Timer; +import bisq.common.UserThread; import bisq.common.taskrunner.TaskRunner; -import java.util.UUID; +import javafx.beans.value.ChangeListener; + +import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; +import static com.google.common.base.Preconditions.checkNotNull; + @Slf4j -public class BuyerSendCounterCurrencyTransferStartedMessage extends TradeTask { +public class BuyerSendCounterCurrencyTransferStartedMessage extends SendMailboxMessageTask { + private static final long MAX_REFRESH_INTERVAL = TimeUnit.HOURS.toMillis(4); + + private ChangeListener listener; + private Timer timer; + private CounterCurrencyTransferStartedMessage counterCurrencyTransferStartedMessage; + public BuyerSendCounterCurrencyTransferStartedMessage(TaskRunner taskHandler, Trade trade) { super(taskHandler, trade); } + @Override + protected TradeMessage getMessage(String tradeId) { + if (counterCurrencyTransferStartedMessage == null) { + AddressEntry payoutAddressEntry = processModel.getBtcWalletService().getOrCreateAddressEntry(tradeId, + AddressEntry.Context.TRADE_PAYOUT); + + // We do not use a real unique ID here as we want to be able to re-send the exact same message in case the + // peer does not respond with an ACK msg in a certain time interval. To avoid that we get dangling mailbox + // messages where only the one which gets processed by the peer would be removed we use the same uid. All + // other data stays the same when we re-send the message at any time later. + String deterministicId = tradeId + processModel.getMyNodeAddress().getFullAddress(); + counterCurrencyTransferStartedMessage = new CounterCurrencyTransferStartedMessage( + tradeId, + payoutAddressEntry.getAddressString(), + processModel.getMyNodeAddress(), + processModel.getPayoutTxSignature(), + trade.getCounterCurrencyTxId(), + trade.getCounterCurrencyExtraData(), + deterministicId + ); + } + return counterCurrencyTransferStartedMessage; + } + + @Override + protected void setStateSent() { + trade.setState(Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG); + } + + @Override + protected void setStateArrived() { + trade.setState(Trade.State.BUYER_SAW_ARRIVED_FIAT_PAYMENT_INITIATED_MSG); + stop(); + } + + @Override + protected void setStateStoredInMailbox() { + trade.setState(Trade.State.BUYER_STORED_IN_MAILBOX_FIAT_PAYMENT_INITIATED_MSG); + start(); + } + + @Override + protected void setStateFault() { + trade.setState(Trade.State.BUYER_SEND_FAILED_FIAT_PAYMENT_INITIATED_MSG); + start(); + } + @Override protected void run() { try { runInterceptHook(); - BtcWalletService walletService = processModel.getBtcWalletService(); - final String id = processModel.getOfferId(); - AddressEntry payoutAddressEntry = walletService.getOrCreateAddressEntry(id, - AddressEntry.Context.TRADE_PAYOUT); - final CounterCurrencyTransferStartedMessage message = new CounterCurrencyTransferStartedMessage( - id, - payoutAddressEntry.getAddressString(), - processModel.getMyNodeAddress(), - processModel.getPayoutTxSignature(), - trade.getCounterCurrencyTxId(), - trade.getCounterCurrencyExtraData(), - UUID.randomUUID().toString() - ); - NodeAddress peersNodeAddress = trade.getTradingPeerNodeAddress(); - log.info("Send {} to peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - trade.setState(Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG); - processModel.getP2PService().sendEncryptedMailboxMessage( - peersNodeAddress, - processModel.getTradingPeer().getPubKeyRing(), - message, - new SendMailboxMessageListener() { - @Override - public void onArrived() { - log.info("{} arrived at peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - trade.setState(Trade.State.BUYER_SAW_ARRIVED_FIAT_PAYMENT_INITIATED_MSG); - complete(); - } - - @Override - public void onStoredInMailbox() { - log.info("{} stored in mailbox for peer {}. tradeId={}, uid={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid()); - trade.setState(Trade.State.BUYER_STORED_IN_MAILBOX_FIAT_PAYMENT_INITIATED_MSG); - complete(); - } - - @Override - public void onFault(String errorMessage) { - log.error("{} failed: Peer {}. tradeId={}, uid={}, errorMessage={}", - message.getClass().getSimpleName(), peersNodeAddress, message.getTradeId(), message.getUid(), errorMessage); - trade.setState(Trade.State.BUYER_SEND_FAILED_FIAT_PAYMENT_INITIATED_MSG); - appendToErrorMessage("Sending message failed: message=" + message + "\nerrorMessage=" + errorMessage); - failed(errorMessage); - } - } - ); + super.run(); } catch (Throwable t) { failed(t); } } + + private void stop() { + if (timer != null) { + timer.stop(); + processModel.getPaymentStartedMessageStateProperty().removeListener(listener); + } + } + + // The listeners ensure we don't get GCed even we have completed the task. + private void start() { + if (timer != null) { + return; + } + + PaymentMethod paymentMethod = checkNotNull(trade.getOffer()).getPaymentMethod(); + // For instant trades with 1 hour we want a short interval, otherwise a few hours should be ok. + long interval = Math.min(paymentMethod.getMaxTradePeriod() / 5, MAX_REFRESH_INTERVAL); + timer = UserThread.runPeriodically(this::run, interval, TimeUnit.MILLISECONDS); + + listener = (observable, oldValue, newValue) -> { + // Once we receive an ACK from our msg we know the peer has received the msg and we stop. + if (newValue == MessageState.ACKNOWLEDGED) { + // We treat a ACK like BUYER_SAW_ARRIVED_FIAT_PAYMENT_INITIATED_MSG + trade.setState(Trade.State.BUYER_SAW_ARRIVED_FIAT_PAYMENT_INITIATED_MSG); + // Ensure listener construction is completed before remove call + UserThread.execute(this::stop); + } + }; + processModel.getPaymentStartedMessageStateProperty().addListener(listener); + } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutSignatureMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutSignatureMessage.java index 3636dbcbfe..a049d0932d 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutSignatureMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutSignatureMessage.java @@ -48,13 +48,14 @@ public class ProcessMediatedPayoutSignatureMessage extends TradeTask { // update to the latest peer address of our peer if the message is correct trade.setTradingPeerNodeAddress(processModel.getTempTradingPeerNodeAddress()); - processModel.removeMailboxMessageAfterProcessing(trade); trade.setMediationResultState(MediationResultState.RECEIVED_SIG_MSG); complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutTxPublishedMessage.java index 6f0f5b3d69..9fdcc4b75c 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutTxPublishedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/ProcessMediatedPayoutTxPublishedMessage.java @@ -74,10 +74,11 @@ public class ProcessMediatedPayoutTxPublishedMessage extends TradeTask { } else { log.info("We got the payout tx already set from BuyerSetupPayoutTxListener and do nothing here. trade ID={}", trade.getId()); } - processModel.removeMailboxMessageAfterProcessing(trade); complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/SendMediatedPayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/SendMediatedPayoutTxPublishedMessage.java index a67ba017a0..8cadcbeaec 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/SendMediatedPayoutTxPublishedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/SendMediatedPayoutTxPublishedMessage.java @@ -21,7 +21,7 @@ import bisq.core.support.dispute.mediation.MediationResultState; import bisq.core.trade.Trade; import bisq.core.trade.messages.MediatedPayoutTxPublishedMessage; import bisq.core.trade.messages.TradeMessage; -import bisq.core.trade.protocol.tasks.SendPayoutTxPublishedMessage; +import bisq.core.trade.protocol.tasks.SendMailboxMessageTask; import bisq.common.taskrunner.TaskRunner; @@ -35,7 +35,7 @@ import static com.google.common.base.Preconditions.checkNotNull; @Slf4j -public class SendMediatedPayoutTxPublishedMessage extends SendPayoutTxPublishedMessage { +public class SendMediatedPayoutTxPublishedMessage extends SendMailboxMessageTask { public SendMediatedPayoutTxPublishedMessage(TaskRunner taskHandler, Trade trade) { super(taskHandler, trade); } @@ -76,6 +76,12 @@ public class SendMediatedPayoutTxPublishedMessage extends SendPayoutTxPublishedM try { runInterceptHook(); + if (trade.getPayoutTx() == null) { + log.error("PayoutTx is null"); + failed("PayoutTx is null"); + return; + } + super.run(); } catch (Throwable t) { failed(t); diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java index cf1f511294..d2a1296445 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java @@ -59,13 +59,13 @@ public class SellerProcessCounterCurrencyTransferStartedMessage extends TradeTas trade.setCounterCurrencyExtraData(counterCurrencyExtraData); } - processModel.removeMailboxMessageAfterProcessing(trade); - trade.setState(Trade.State.SELLER_RECEIVED_FIAT_PAYMENT_INITIATED_MSG); complete(); } catch (Throwable t) { failed(t); + } finally { + processModel.removeMailboxMessageAfterProcessing(trade); } } } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendPayoutTxPublishedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendPayoutTxPublishedMessage.java index 3bda43fa86..db73f1910a 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendPayoutTxPublishedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendPayoutTxPublishedMessage.java @@ -17,23 +17,27 @@ package bisq.core.trade.protocol.tasks.seller; +import bisq.core.account.sign.SignedWitness; +import bisq.core.account.witness.AccountAgeWitnessService; import bisq.core.trade.Trade; import bisq.core.trade.messages.PayoutTxPublishedMessage; import bisq.core.trade.messages.TradeMessage; -import bisq.core.trade.protocol.tasks.SendPayoutTxPublishedMessage; +import bisq.core.trade.protocol.tasks.SendMailboxMessageTask; import bisq.common.taskrunner.TaskRunner; import org.bitcoinj.core.Transaction; -import java.util.UUID; - +import lombok.EqualsAndHashCode; import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkNotNull; +@EqualsAndHashCode(callSuper = true) @Slf4j -public class SellerSendPayoutTxPublishedMessage extends SendPayoutTxPublishedMessage { +public class SellerSendPayoutTxPublishedMessage extends SendMailboxMessageTask { + SignedWitness signedWitness = null; + public SellerSendPayoutTxPublishedMessage(TaskRunner taskHandler, Trade trade) { super(taskHandler, trade); } @@ -41,32 +45,47 @@ public class SellerSendPayoutTxPublishedMessage extends SendPayoutTxPublishedMes @Override protected TradeMessage getMessage(String id) { Transaction payoutTx = checkNotNull(trade.getPayoutTx(), "trade.getPayoutTx() must not be null"); + + AccountAgeWitnessService accountAgeWitnessService = processModel.getAccountAgeWitnessService(); + if (accountAgeWitnessService.isSignWitnessTrade(trade)) { + // Broadcast is done in accountAgeWitness domain. + accountAgeWitnessService.traderSignAndPublishPeersAccountAgeWitness(trade).ifPresent(witness -> signedWitness = witness); + } + return new PayoutTxPublishedMessage( id, payoutTx.bitcoinSerialize(), processModel.getMyNodeAddress(), - UUID.randomUUID().toString() + signedWitness ); } @Override protected void setStateSent() { trade.setState(Trade.State.SELLER_SENT_PAYOUT_TX_PUBLISHED_MSG); + log.info("Sent PayoutTxPublishedMessage: tradeId={} at peer {} SignedWitness {}", + trade.getId(), trade.getTradingPeerNodeAddress(), signedWitness); } @Override protected void setStateArrived() { trade.setState(Trade.State.SELLER_SAW_ARRIVED_PAYOUT_TX_PUBLISHED_MSG); + log.info("PayoutTxPublishedMessage arrived: tradeId={} at peer {} SignedWitness {}", + trade.getId(), trade.getTradingPeerNodeAddress(), signedWitness); } @Override protected void setStateStoredInMailbox() { trade.setState(Trade.State.SELLER_STORED_IN_MAILBOX_PAYOUT_TX_PUBLISHED_MSG); + log.info("PayoutTxPublishedMessage storedInMailbox: tradeId={} at peer {} SignedWitness {}", + trade.getId(), trade.getTradingPeerNodeAddress(), signedWitness); } @Override protected void setStateFault() { trade.setState(Trade.State.SELLER_SEND_FAILED_PAYOUT_TX_PUBLISHED_MSG); + log.error("PayoutTxPublishedMessage failed: tradeId={} at peer {} SignedWitness {}", + trade.getId(), trade.getTradingPeerNodeAddress(), signedWitness); } @Override @@ -74,6 +93,12 @@ public class SellerSendPayoutTxPublishedMessage extends SendPayoutTxPublishedMes try { runInterceptHook(); + if (trade.getPayoutTx() == null) { + log.error("PayoutTx is null"); + failed("PayoutTx is null"); + return; + } + super.run(); } catch (Throwable t) { failed(t); diff --git a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java index 338a9b3a70..55a6e8879b 100644 --- a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java @@ -354,7 +354,7 @@ public class SignedWitnessServiceTest { when(keyRing.getSignatureKeyPair()).thenReturn(signerKeyPair); AccountAgeWitness accountAgeWitness = new AccountAgeWitness(account1DataHash, accountCreationTime); - signedWitnessService.signAccountAgeWitness(Coin.ZERO, accountAgeWitness, peerKeyPair.getPublic()); + signedWitnessService.signAndPublishAccountAgeWitness(Coin.ZERO, accountAgeWitness, peerKeyPair.getPublic()); verify(p2pService, never()).addPersistableNetworkPayload(any(PersistableNetworkPayload.class), anyBoolean()); } @@ -370,7 +370,7 @@ public class SignedWitnessServiceTest { AccountAgeWitness accountAgeWitness = new AccountAgeWitness(account1DataHash, accountCreationTime); - signedWitnessService.signAccountAgeWitness(SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness, peerKeyPair.getPublic()); + signedWitnessService.signAndPublishAccountAgeWitness(SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness, peerKeyPair.getPublic()); verify(p2pService, times(1)).addPersistableNetworkPayload(any(PersistableNetworkPayload.class), anyBoolean()); } diff --git a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java index 2b2d11e562..10840861dd 100644 --- a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java @@ -301,7 +301,7 @@ public class AccountAgeWitnessServiceTest { signAccountAgeWitness(aew2, pubKeyRing2.getSignaturePubKey(), aew2.getDate(), user1KeyRing); // user2 signs user3 signAccountAgeWitness(aew3, pubKeyRing3.getSignaturePubKey(), aew3.getDate(), user2KeyRing); - signedWitnessService.signAccountAgeWitness(SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING, aew2, + signedWitnessService.signAndPublishAccountAgeWitness(SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING, aew2, pubKeyRing2.getSignaturePubKey()); assertTrue(service.accountIsSigner(aew1)); assertTrue(service.accountIsSigner(aew2)); diff --git a/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java b/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java index 2cd3dc1ff3..a9b5f3bacc 100644 --- a/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java +++ b/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java @@ -130,7 +130,7 @@ class FiatAccountsDataModel extends ActivatableDataModel { user.addPaymentAccount(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); - accountAgeWitnessService.signSameNameAccounts(); + accountAgeWitnessService.signAndPublishSameNameAccounts(); } public boolean onDeleteAccount(PaymentAccount paymentAccount) { diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java index ab0d06c7ea..2a4e584f79 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java @@ -50,13 +50,10 @@ import bisq.core.trade.BuyerTrade; import bisq.core.trade.SellerTrade; import bisq.core.trade.Trade; import bisq.core.trade.TradeManager; -import bisq.core.trade.messages.RefreshTradeStateRequest; import bisq.core.user.Preferences; import bisq.core.util.FormattingUtils; -import bisq.network.p2p.NodeAddress; import bisq.network.p2p.P2PService; -import bisq.network.p2p.SendMailboxMessageListener; import bisq.common.crypto.PubKeyRing; import bisq.common.handlers.ErrorMessageHandler; @@ -81,7 +78,6 @@ import javafx.collections.ObservableList; import org.bouncycastle.crypto.params.KeyParameter; -import java.util.UUID; import java.util.stream.Collectors; import lombok.Getter; @@ -237,43 +233,6 @@ public class PendingTradesDataModel extends ActivatableDataModel { tradeManager.addTradeToFailedTrades(getTrade()); } - // Ask counterparty to resend last action (in case message was lost) - public void refreshTradeState() { - Trade trade = getTrade(); - if (trade == null || !trade.allowedRefresh()) return; - - trade.logRefresh(); - NodeAddress tradingPeerNodeAddress = trade.getTradingPeerNodeAddress(); - - RefreshTradeStateRequest refreshReq = new RefreshTradeStateRequest(UUID.randomUUID().toString(), - trade.getId(), - tradingPeerNodeAddress); - p2PService.sendEncryptedMailboxMessage( - tradingPeerNodeAddress, - trade.getProcessModel().getTradingPeer().getPubKeyRing(), - refreshReq, - new SendMailboxMessageListener() { - @Override - public void onArrived() { - log.info("SendMailboxMessageListener onArrived tradeId={} at peer {}", - trade.getId(), tradingPeerNodeAddress); - } - - @Override - public void onStoredInMailbox() { - log.info("SendMailboxMessageListener onStoredInMailbox tradeId={} at peer {}", - trade.getId(), tradingPeerNodeAddress); - } - - @Override - public void onFault(String errorMessage) { - log.error("SendMailboxMessageListener onFault tradeId={} at peer {}", - trade.getId(), tradingPeerNodeAddress); - } - } - ); - tradeManager.persistTrades(); - } /////////////////////////////////////////////////////////////////////////////////////////// // Getters diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java index 3776b3f1a7..d211997f8a 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java @@ -216,8 +216,6 @@ public class PendingTradesView extends ActivatableViewAndModel onRefreshButton()); - - GridPane.setRowIndex(refreshButtonPane, ++gridRow); - GridPane.setColumnIndex(refreshButtonPane, 0); - GridPane.setColumnSpan(refreshButtonPane, 2); - gridPane.getChildren().add(refreshButtonPane); - } - - @Override public void activate() { super.activate(); - activateRefreshButton(); } @Override public void deactivate() { super.deactivate(); - deActivateRefreshButtonTimer(); - } - - private void activateRefreshButton() { - checkNotNull(model.dataModel.getTrade(), "No trade found"); - - Trade trade = model.dataModel.getTrade(); - var timeToNextRefresh = - trade.getLastRefreshRequestDate() + trade.getRefreshInterval() - new Date().getTime(); - if (timeToNextRefresh <= 0) { - refreshButtonPane.setVisible(true); - } else { - refreshButtonPane.setVisible(false); - timer = UserThread.runAfter(this::activateRefreshButton, timeToNextRefresh, TimeUnit.MILLISECONDS); - } - } - - private void deActivateRefreshButtonTimer() { - if (timer != null) { - timer.stop(); - } - } - - private void onRefreshButton() { - model.dataModel.refreshTradeState(); - activateRefreshButton(); } diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 2f334e06e0..1cb5dff3a8 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -76,7 +76,7 @@ message NetworkEnvelope { DepositTxAndDelayedPayoutTxMessage deposit_tx_and_delayed_payout_tx_message = 48; PeerPublishedDelayedPayoutTxMessage peer_published_delayed_payout_tx_message = 49; - RefreshTradeStateRequest refresh_trade_state_request = 50; + RefreshTradeStateRequest refresh_trade_state_request = 50 [deprecated = true]; TraderSignedWitnessMessage trader_signed_witness_message = 51; } } @@ -309,6 +309,7 @@ message PayoutTxPublishedMessage { bytes payout_tx = 2; NodeAddress sender_node_address = 3; string uid = 4; + SignedWitness signed_witness = 5; // Added in v1.4.0 } message MediatedPayoutTxPublishedMessage { @@ -325,17 +326,19 @@ message MediatedPayoutTxSignatureMessage { NodeAddress sender_node_address = 4; } +// Deprecated since 1.4.0 message RefreshTradeStateRequest { - string uid = 1; - string trade_id = 2; - NodeAddress sender_node_address = 3; + string uid = 1 [deprecated = true]; + string trade_id = 2 [deprecated = true]; + NodeAddress sender_node_address = 3 [deprecated = true]; } +// Deprecated since 1.4.0 message TraderSignedWitnessMessage { - string uid = 1; - string trade_id = 2; - NodeAddress sender_node_address = 3; - SignedWitness signed_witness = 4; + string uid = 1 [deprecated = true]; + string trade_id = 2 [deprecated = true]; + NodeAddress sender_node_address = 3 [deprecated = true]; + SignedWitness signed_witness = 4 [deprecated = true]; } // dispute