From c153afff677a5131ec9dfd46b770f2762fc3736a Mon Sep 17 00:00:00 2001
From: woodser <woodser@protonmail.com>
Date: Sat, 1 Oct 2022 07:47:18 -0400
Subject: [PATCH] arbitrator does not share payment account key until after
first confirmation (#457)
use payout address from contract instead of PaymentSentMessage
---
.../core/trade/messages/PaymentSentMessage.java | 8 --------
.../core/trade/protocol/ArbitratorProtocol.java | 2 +-
...ArbitratorProcessPaymentAccountKeyRequest.java | 6 ++++++
.../tasks/BuyerSendPaymentSentMessage.java | 8 --------
.../tasks/ProcessSignContractRequest.java | 1 -
.../tasks/ProcessSignContractResponse.java | 2 +-
.../tasks/SellerProcessPaymentSentMessage.java | 4 ++--
proto/src/main/proto/pb.proto | 15 +++++++--------
8 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/core/src/main/java/bisq/core/trade/messages/PaymentSentMessage.java b/core/src/main/java/bisq/core/trade/messages/PaymentSentMessage.java
index fcf5674e74..1342237c1f 100644
--- a/core/src/main/java/bisq/core/trade/messages/PaymentSentMessage.java
+++ b/core/src/main/java/bisq/core/trade/messages/PaymentSentMessage.java
@@ -32,7 +32,6 @@ import javax.annotation.Nullable;
@EqualsAndHashCode(callSuper = true)
@Value
public final class PaymentSentMessage extends TradeMailboxMessage {
- private final String buyerPayoutAddress;
private final NodeAddress senderNodeAddress;
@Nullable
private final String counterCurrencyTxId;
@@ -49,7 +48,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
private String counterCurrencyExtraData;
public PaymentSentMessage(String tradeId,
- String buyerPayoutAddress,
NodeAddress senderNodeAddress,
@Nullable String counterCurrencyTxId,
@Nullable String counterCurrencyExtraData,
@@ -58,7 +56,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
String updatedMultisigHex,
@Nullable byte[] paymentAccountKey) {
this(tradeId,
- buyerPayoutAddress,
senderNodeAddress,
counterCurrencyTxId,
counterCurrencyExtraData,
@@ -75,7 +72,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
///////////////////////////////////////////////////////////////////////////////////////////
private PaymentSentMessage(String tradeId,
- String buyerPayoutAddress,
NodeAddress senderNodeAddress,
@Nullable String counterCurrencyTxId,
@Nullable String counterCurrencyExtraData,
@@ -85,7 +81,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
@Nullable String updatedMultisigHex,
@Nullable byte[] paymentAccountKey) {
super(messageVersion, tradeId, uid);
- this.buyerPayoutAddress = buyerPayoutAddress;
this.senderNodeAddress = senderNodeAddress;
this.counterCurrencyTxId = counterCurrencyTxId;
this.counterCurrencyExtraData = counterCurrencyExtraData;
@@ -98,7 +93,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
public protobuf.NetworkEnvelope toProtoNetworkEnvelope() {
final protobuf.PaymentSentMessage.Builder builder = protobuf.PaymentSentMessage.newBuilder();
builder.setTradeId(tradeId)
- .setBuyerPayoutAddress(buyerPayoutAddress)
.setSenderNodeAddress(senderNodeAddress.toProtoMessage())
.setUid(uid);
@@ -114,7 +108,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
public static PaymentSentMessage fromProto(protobuf.PaymentSentMessage proto,
String messageVersion) {
return new PaymentSentMessage(proto.getTradeId(),
- proto.getBuyerPayoutAddress(),
NodeAddress.fromProto(proto.getSenderNodeAddress()),
ProtoUtil.stringOrNullFromProto(proto.getCounterCurrencyTxId()),
ProtoUtil.stringOrNullFromProto(proto.getCounterCurrencyExtraData()),
@@ -130,7 +123,6 @@ public final class PaymentSentMessage extends TradeMailboxMessage {
@Override
public String toString() {
return "PaymentSentMessage{" +
- "\n buyerPayoutAddress='" + buyerPayoutAddress + '\'' +
",\n senderNodeAddress=" + senderNodeAddress +
",\n counterCurrencyTxId=" + counterCurrencyTxId +
",\n counterCurrencyExtraData=" + counterCurrencyExtraData +
diff --git a/core/src/main/java/bisq/core/trade/protocol/ArbitratorProtocol.java b/core/src/main/java/bisq/core/trade/protocol/ArbitratorProtocol.java
index 3cb1085570..d28e792579 100644
--- a/core/src/main/java/bisq/core/trade/protocol/ArbitratorProtocol.java
+++ b/core/src/main/java/bisq/core/trade/protocol/ArbitratorProtocol.java
@@ -154,7 +154,7 @@ public class ArbitratorProtocol extends DisputeProtocol {
latchTrade();
Validator.checkTradeId(processModel.getOfferId(), request);
processModel.setTradeMessage(request);
- expect(phase(Trade.Phase.DEPOSITS_PUBLISHED)
+ expect(anyPhase(Trade.Phase.DEPOSITS_PUBLISHED, Trade.Phase.DEPOSITS_CONFIRMED, Trade.Phase.DEPOSITS_UNLOCKED)
.with(request)
.from(peer))
.setup(tasks(
diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ArbitratorProcessPaymentAccountKeyRequest.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ArbitratorProcessPaymentAccountKeyRequest.java
index 9b5fe2cf08..df12736630 100644
--- a/core/src/main/java/bisq/core/trade/protocol/tasks/ArbitratorProcessPaymentAccountKeyRequest.java
+++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ArbitratorProcessPaymentAccountKeyRequest.java
@@ -41,6 +41,12 @@ public class ArbitratorProcessPaymentAccountKeyRequest extends TradeTask {
try {
runInterceptHook();
+ // ensure deposit txs confirmed
+ trade.listenForDepositTxs();
+ if (trade.getPhase().ordinal() < Trade.Phase.DEPOSITS_CONFIRMED.ordinal()) {
+ throw new RuntimeException("Arbitrator refusing payment account key request for trade " + trade.getId() + " because the deposit txs have not confirmed");
+ }
+
// create response for buyer with key to decrypt seller's payment account payload
PaymentAccountKeyResponse response = new PaymentAccountKeyResponse(
trade.getId(),
diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/BuyerSendPaymentSentMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/BuyerSendPaymentSentMessage.java
index 91874c1357..334d6c3d5a 100644
--- a/core/src/main/java/bisq/core/trade/protocol/tasks/BuyerSendPaymentSentMessage.java
+++ b/core/src/main/java/bisq/core/trade/protocol/tasks/BuyerSendPaymentSentMessage.java
@@ -17,8 +17,6 @@
package bisq.core.trade.protocol.tasks;
-import bisq.core.btc.model.XmrAddressEntry;
-import bisq.core.btc.wallet.XmrWalletService;
import bisq.core.network.MessageState;
import bisq.core.trade.Trade;
import bisq.core.trade.messages.PaymentSentMessage;
@@ -53,11 +51,6 @@ public class BuyerSendPaymentSentMessage extends SendMailboxMessageTask {
protected TradeMailboxMessage getTradeMailboxMessage(String tradeId) {
if (message == null) {
- // gather relevant info
- XmrWalletService walletService = processModel.getProvider().getXmrWalletService();
- final String id = processModel.getOfferId();
- XmrAddressEntry payoutAddressEntry = walletService.getOrCreateAddressEntry(id, XmrAddressEntry.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
@@ -65,7 +58,6 @@ public class BuyerSendPaymentSentMessage extends SendMailboxMessageTask {
String deterministicId = tradeId + processModel.getMyNodeAddress().getFullAddress();
message = new PaymentSentMessage(
tradeId,
- payoutAddressEntry.getAddressString(),
processModel.getMyNodeAddress(),
trade.getCounterCurrencyTxId(),
trade.getCounterCurrencyExtraData(),
diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractRequest.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractRequest.java
index 765d29cef5..fa56e56a7f 100644
--- a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractRequest.java
+++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractRequest.java
@@ -68,7 +68,6 @@ public class ProcessSignContractRequest extends TradeTask {
trader.setPayoutAddressString(request.getPayoutAddress());
// sign contract only when both deposit txs hashes known
- // TODO (woodser): synchronize contract creation; both requests received at the same time
// TODO (woodser): remove makerDepositTxId and takerDepositTxId from Trade
if (processModel.getMaker().getDepositTxHash() == null || processModel.getTaker().getDepositTxHash() == null) {
complete();
diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractResponse.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractResponse.java
index 483d099575..c6ba9186c9 100644
--- a/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractResponse.java
+++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractResponse.java
@@ -99,7 +99,7 @@ public class ProcessSignContractResponse extends TradeTask {
@Override
public void onArrived() {
log.info("{} arrived: arbitrator={}; offerId={}; uid={}", request.getClass().getSimpleName(), trade.getArbitratorNodeAddress(), trade.getId(), request.getUid());
- trade.setState(Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST);
+ trade.setStateIfValidTransitionTo(Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST);
processModel.getTradeManager().requestPersistence();
complete();
}
diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/SellerProcessPaymentSentMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/SellerProcessPaymentSentMessage.java
index a84561c5d0..09dbeb3859 100644
--- a/core/src/main/java/bisq/core/trade/protocol/tasks/SellerProcessPaymentSentMessage.java
+++ b/core/src/main/java/bisq/core/trade/protocol/tasks/SellerProcessPaymentSentMessage.java
@@ -42,11 +42,11 @@ public class SellerProcessPaymentSentMessage extends TradeTask {
Validator.checkTradeId(processModel.getOfferId(), message);
checkNotNull(message);
- trade.getBuyer().setPayoutAddressString(Validator.nonEmptyStringOf(message.getBuyerPayoutAddress())); // TODO (woodser): verify against contract
+ // store buyer info
trade.getBuyer().setPayoutTxHex(message.getPayoutTxHex());
trade.getBuyer().setUpdatedMultisigHex(message.getUpdatedMultisigHex());
- // decrypt peer's payment account payload
+ // decrypt buyer's payment account payload
trade.decryptPeersPaymentAccountPayload(message.getPaymentAccountKey());
// sync and update multisig wallet
diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto
index 3b2f63cc01..430146e56c 100644
--- a/proto/src/main/proto/pb.proto
+++ b/proto/src/main/proto/pb.proto
@@ -440,14 +440,13 @@ message FinalizePayoutTxRequest {
message PaymentSentMessage {
string trade_id = 1;
- string buyer_payout_address = 2;
- NodeAddress sender_node_address = 3;
- string counter_currency_tx_id = 4;
- string uid = 5;
- string counter_currency_extra_data = 6;
- string payout_tx_hex = 7;
- string updated_multisig_hex = 8;
- bytes payment_account_key = 9;
+ NodeAddress sender_node_address = 2;
+ string counter_currency_tx_id = 3;
+ string uid = 4;
+ string counter_currency_extra_data = 5;
+ string payout_tx_hex = 6;
+ string updated_multisig_hex = 7;
+ bytes payment_account_key = 8;
}
message PaymentReceivedMessage {