From 4ab75b656d6e712c3056e74aafa74163b7a39e57 Mon Sep 17 00:00:00 2001 From: Christoph Atteneder Date: Thu, 3 Dec 2020 12:57:43 +0100 Subject: [PATCH 01/21] Bump version number for v1.5.1 --- build.gradle | 2 +- common/src/main/java/bisq/common/app/Version.java | 2 +- desktop/package/linux/Dockerfile | 2 +- desktop/package/linux/package.sh | 2 +- desktop/package/linux/release.sh | 2 +- desktop/package/macosx/Info.plist | 4 ++-- desktop/package/macosx/create_app.sh | 2 +- desktop/package/macosx/finalize.sh | 2 +- desktop/package/macosx/insert_snapshot_version.sh | 2 +- desktop/package/macosx/replace_version_number.sh | 4 ++-- desktop/package/windows/package.bat | 2 +- desktop/package/windows/release.bat | 2 +- relay/src/main/resources/version.txt | 2 +- seednode/src/main/java/bisq/seednode/SeedNodeMain.java | 2 +- 14 files changed, 16 insertions(+), 16 deletions(-) diff --git a/build.gradle b/build.gradle index 5b0a9ac6e8e..f40319ce85e 100644 --- a/build.gradle +++ b/build.gradle @@ -386,7 +386,7 @@ configure(project(':desktop')) { apply plugin: 'witness' apply from: '../gradle/witness/gradle-witness.gradle' - version = '1.5.0' + version = '1.5.1' mainClassName = 'bisq.desktop.app.BisqAppMain' diff --git a/common/src/main/java/bisq/common/app/Version.java b/common/src/main/java/bisq/common/app/Version.java index 470d0263568..4acaab88eb4 100644 --- a/common/src/main/java/bisq/common/app/Version.java +++ b/common/src/main/java/bisq/common/app/Version.java @@ -30,7 +30,7 @@ public class Version { // VERSION = 0.5.0 introduces proto buffer for the P2P network and local DB and is a not backward compatible update // Therefore all sub versions start again with 1 // We use semantic versioning with major, minor and patch - public static final String VERSION = "1.5.0"; + public static final String VERSION = "1.5.1"; /** * Holds a list of the tagged resource files for optimizing the getData requests. diff --git a/desktop/package/linux/Dockerfile b/desktop/package/linux/Dockerfile index d5e4f4e5ba7..8aee1e69c03 100644 --- a/desktop/package/linux/Dockerfile +++ b/desktop/package/linux/Dockerfile @@ -8,7 +8,7 @@ # pull base image FROM openjdk:8-jdk -ENV version 1.5.0 +ENV version 1.5.1 RUN apt-get update && apt-get install -y --no-install-recommends openjfx && rm -rf /var/lib/apt/lists/* && apt-get install -y vim fakeroot diff --git a/desktop/package/linux/package.sh b/desktop/package/linux/package.sh index b3b22473c92..c95027f33ae 100755 --- a/desktop/package/linux/package.sh +++ b/desktop/package/linux/package.sh @@ -6,7 +6,7 @@ # - Update version below # - Ensure JAVA_HOME below is pointing to OracleJDK 10 directory -version=1.5.0 +version=1.5.1 version_base=$(echo $version | awk -F'[_-]' '{print $1}') if [ ! -f "$JAVA_HOME/bin/javapackager" ]; then if [ -d "/usr/lib/jvm/jdk-10.0.2" ]; then diff --git a/desktop/package/linux/release.sh b/desktop/package/linux/release.sh index 86b518251b2..49b5964d34b 100755 --- a/desktop/package/linux/release.sh +++ b/desktop/package/linux/release.sh @@ -4,7 +4,7 @@ # Prior to running this script: # - Update version below -version=1.5.0 +version=1.5.1 base_dir=$( cd "$(dirname "$0")" ; pwd -P )/../../.. package_dir=$base_dir/desktop/package release_dir=$base_dir/desktop/release/$version diff --git a/desktop/package/macosx/Info.plist b/desktop/package/macosx/Info.plist index 999eb7c82c5..0aebad95322 100644 --- a/desktop/package/macosx/Info.plist +++ b/desktop/package/macosx/Info.plist @@ -5,10 +5,10 @@ CFBundleVersion - 1.5.0 + 1.5.1 CFBundleShortVersionString - 1.5.0 + 1.5.1 CFBundleExecutable Bisq diff --git a/desktop/package/macosx/create_app.sh b/desktop/package/macosx/create_app.sh index 53d76c35289..6c405a84241 100755 --- a/desktop/package/macosx/create_app.sh +++ b/desktop/package/macosx/create_app.sh @@ -6,7 +6,7 @@ mkdir -p deploy set -e -version="1.5.0" +version="1.5.1" cd .. ./gradlew :desktop:build -x test shadowJar diff --git a/desktop/package/macosx/finalize.sh b/desktop/package/macosx/finalize.sh index e756e84d329..55b2181508c 100755 --- a/desktop/package/macosx/finalize.sh +++ b/desktop/package/macosx/finalize.sh @@ -2,7 +2,7 @@ cd ../../ -version="1.5.0" +version="1.5.1" target_dir="releases/$version" diff --git a/desktop/package/macosx/insert_snapshot_version.sh b/desktop/package/macosx/insert_snapshot_version.sh index 9670cc44685..604e5575589 100755 --- a/desktop/package/macosx/insert_snapshot_version.sh +++ b/desktop/package/macosx/insert_snapshot_version.sh @@ -2,7 +2,7 @@ cd $(dirname $0)/../../../ -version=1.4.2 +version=1.5.0 find . -type f \( -name "finalize.sh" \ -o -name "create_app.sh" \ diff --git a/desktop/package/macosx/replace_version_number.sh b/desktop/package/macosx/replace_version_number.sh index dd941245260..be7cca70d71 100755 --- a/desktop/package/macosx/replace_version_number.sh +++ b/desktop/package/macosx/replace_version_number.sh @@ -2,8 +2,8 @@ cd $(dirname $0)/../../../ -oldVersion=1.4.2 -newVersion=1.5.0 +oldVersion=1.5.0 +newVersion=1.5.1 find . -type f \( -name "finalize.sh" \ -o -name "create_app.sh" \ diff --git a/desktop/package/windows/package.bat b/desktop/package/windows/package.bat index 3e41c4912b3..3c86e20dd80 100644 --- a/desktop/package/windows/package.bat +++ b/desktop/package/windows/package.bat @@ -11,7 +11,7 @@ @echo off -set version=1.5.0 +set version=1.5.1 if not exist "%JAVA_HOME%\bin\javapackager.exe" ( if not exist "%ProgramFiles%\Java\jdk-10.0.2" ( echo Javapackager not found. Update JAVA_HOME variable to point to OracleJDK. diff --git a/desktop/package/windows/release.bat b/desktop/package/windows/release.bat index 0bd363c22d3..7b3caef996b 100644 --- a/desktop/package/windows/release.bat +++ b/desktop/package/windows/release.bat @@ -6,7 +6,7 @@ @echo off -set version=1.5.0 +set version=1.5.1 set release_dir=%~dp0..\..\..\releases\%version% set package_dir=%~dp0.. diff --git a/relay/src/main/resources/version.txt b/relay/src/main/resources/version.txt index bc80560fad6..26ca594609a 100644 --- a/relay/src/main/resources/version.txt +++ b/relay/src/main/resources/version.txt @@ -1 +1 @@ -1.5.0 +1.5.1 diff --git a/seednode/src/main/java/bisq/seednode/SeedNodeMain.java b/seednode/src/main/java/bisq/seednode/SeedNodeMain.java index 5a871e7471e..a6e04b5009f 100644 --- a/seednode/src/main/java/bisq/seednode/SeedNodeMain.java +++ b/seednode/src/main/java/bisq/seednode/SeedNodeMain.java @@ -33,7 +33,7 @@ @Slf4j public class SeedNodeMain extends ExecutableForAppWithP2p { - private static final String VERSION = "1.5.0"; + private static final String VERSION = "1.5.1"; private SeedNode seedNode; public SeedNodeMain() { From a39f2e8fcb1867eeafe155eaed10033c98157fe8 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 09:54:43 -0500 Subject: [PATCH 02/21] Apply project format rules (no code change) --- .../core/btc/wallet/BtcWalletService.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index 3d45c491878..d5726f10f0f 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -254,8 +254,8 @@ private Transaction completePreparedProposalTx(Transaction feeTx, byte[] opRetur sendRequest.signInputs = false; sendRequest.fee = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4); + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4); sendRequest.feePerKb = Coin.ZERO; sendRequest.ensureMinRequiredFee = false; @@ -274,8 +274,8 @@ private Transaction completePreparedProposalTx(Transaction feeTx, byte[] opRetur numSegwitInputs = numInputs.second; txVsizeWithUnsignedInputs = resultTx.getVsize(); long estimatedFeeAsLong = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4).value; + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4).value; // calculated fee must be inside of a tolerance range with tx fee isFeeOutsideTolerance = Math.abs(resultTx.getFee().value - estimatedFeeAsLong) > 1000; @@ -374,8 +374,8 @@ private Transaction addInputsForMinerFee(Transaction preparedTx, sendRequest.signInputs = false; sendRequest.fee = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4); + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4); sendRequest.feePerKb = Coin.ZERO; sendRequest.ensureMinRequiredFee = false; @@ -393,8 +393,8 @@ private Transaction addInputsForMinerFee(Transaction preparedTx, numSegwitInputs = numInputs.second; txVsizeWithUnsignedInputs = resultTx.getVsize(); final long estimatedFeeAsLong = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4).value; + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4).value; // calculated fee must be inside of a tolerance range with tx fee isFeeOutsideTolerance = Math.abs(resultTx.getFee().value - estimatedFeeAsLong) > 1000; } @@ -532,8 +532,8 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, sendRequest.signInputs = false; sendRequest.fee = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4); + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4); sendRequest.feePerKb = Coin.ZERO; sendRequest.ensureMinRequiredFee = false; @@ -558,8 +558,8 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, numSegwitInputs = numInputs.second; txVsizeWithUnsignedInputs = resultTx.getVsize(); final long estimatedFeeAsLong = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4).value; + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4).value; // calculated fee must be inside of a tolerance range with tx fee isFeeOutsideTolerance = Math.abs(resultTx.getFee().value - estimatedFeeAsLong) > 1000; } @@ -583,7 +583,7 @@ private Tuple2 getNumInputs(Transaction tx) { for (TransactionInput input : tx.getInputs()) { TransactionOutput connectedOutput = input.getConnectedOutput(); if (connectedOutput == null || ScriptPattern.isP2PKH(connectedOutput.getScriptPubKey()) || - ScriptPattern.isP2PK(connectedOutput.getScriptPubKey())) { + ScriptPattern.isP2PK(connectedOutput.getScriptPubKey())) { // If connectedOutput is null, we don't know here the input type. To avoid underpaying fees, // we treat it as a legacy input which will result in a higher fee estimation. numLegacyInputs++; From 29d757b74196c803e967fe4181d764bb11945cf0 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 10:01:01 -0500 Subject: [PATCH 03/21] - revent that a wrong tx is set as deposit tx We check if the txIds of the inputs matches our maker fee tx and taker fee tx and if the depositTxAddress we use for the confidence lookup is use as an output address. This prevents that past txs which have the our depositTxAddress as input or output (deposit or payout txs) could be interpreted as our deposit tx. This happened because if a bug which caused re-use of the Multisig address entries and if both traders use the same key for multiple trades the depositTxAddress would be the same. We fix that bug as well but we also need to avoid that past already used addresses might be taken again (the Multisig flag got reverted to available in the address entry). - Add check to swapTradeEntryToAvailableEntry to not swap MULTI_SIG entries. - Remove swap for MULTI_SIG entries at resetAddressEntriesForPendingTrade - Add check to swapToAvailable to not swap MULTI_SIG entries. - Remove swaps for MULTI_SIG entries - Add setCoinLockedInMultiSigAddressEntry method - Make coinLockedInMultiSig final and remove setter but use it in constructor. - Rename getCoinLockedInMultiSig to getCoinLockedInMultiSigAsCoin We use an immutable list when operating on AddressEntry so changes on the object would not be reflected in the list. The only mutable field (beside non critical cache fields) is the keyPair. Might be good to refactor that as well at some point. - Add setCoinLockedInMultiSigAddressEntrymethods - Apply API changes: -- resetCoinLockedInMultiSigAddressEntry -- setCoinLockedInMultiSigAddressEntry -- renamed methods --- .../main/java/bisq/core/app/BisqSetup.java | 2 +- .../src/main/java/bisq/core/btc/Balances.java | 2 +- .../bisq/core/btc/model/AddressEntry.java | 50 ++++++++------ .../bisq/core/btc/model/AddressEntryList.java | 27 ++++++++ .../core/btc/wallet/BtcWalletService.java | 32 ++++++++- .../protocol/tasks/SetupPayoutTxListener.java | 8 +-- .../arbitration/PublishedDelayedPayoutTx.java | 3 +- .../BuyerProcessPayoutTxPublishedMessage.java | 3 +- .../buyer/BuyerSetupDepositTxListener.java | 67 ++++++++++++++++++- .../BuyerAsMakerCreatesAndSignsDepositTx.java | 2 +- .../BuyerAsTakerSignsDepositTx.java | 3 +- .../mediation/FinalizeMediatedPayoutTx.java | 2 +- ...ocessMediatedPayoutTxPublishedMessage.java | 3 +- .../seller/SellerSignAndFinalizePayoutTx.java | 2 +- ...SellerAsMakerCreatesUnsignedDepositTx.java | 3 +- .../SellerAsTakerSignsDepositTx.java | 3 +- .../main/funds/locked/LockedListItem.java | 2 +- 17 files changed, 170 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index 925b5e793a1..f6ef065f2f6 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -467,7 +467,7 @@ private void checkForLockedUpFunds() { .filter(e -> setOfAllTradeIds.contains(e.getOfferId()) && e.getContext() == AddressEntry.Context.MULTI_SIG) .forEach(e -> { - Coin balance = e.getCoinLockedInMultiSig(); + Coin balance = e.getCoinLockedInMultiSigAsCoin(); if (balance.isPositive()) { String message = Res.get("popup.warning.lockedUpFunds", formatter.formatCoinWithCode(balance), e.getAddressString(), e.getOfferId()); diff --git a/core/src/main/java/bisq/core/btc/Balances.java b/core/src/main/java/bisq/core/btc/Balances.java index e162d547075..5d3ddd4e437 100644 --- a/core/src/main/java/bisq/core/btc/Balances.java +++ b/core/src/main/java/bisq/core/btc/Balances.java @@ -124,7 +124,7 @@ private void updateLockedBalance() { long sum = lockedTrades.map(trade -> btcWalletService.getAddressEntry(trade.getId(), AddressEntry.Context.MULTI_SIG) .orElse(null)) .filter(Objects::nonNull) - .mapToLong(addressEntry -> addressEntry.getCoinLockedInMultiSig().getValue()) + .mapToLong(AddressEntry::getCoinLockedInMultiSig) .sum(); lockedBalance.set(Coin.valueOf(sum)); } diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntry.java b/core/src/main/java/bisq/core/btc/model/AddressEntry.java index 5348950b391..c8804fa29a1 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -71,16 +71,19 @@ public enum Context { private final byte[] pubKey; @Getter private final byte[] pubKeyHash; - - private long coinLockedInMultiSig; - @Getter - private boolean segwit; + private final long coinLockedInMultiSig; + @Getter + private final boolean segwit; + // Not an immutable field. Set at startup once wallet is ready and at encrypting/decrypting wallet. @Nullable transient private DeterministicKey keyPair; + + // Only used as cache @Nullable transient private Address address; + // Only used as cache @Nullable transient private String addressString; @@ -93,16 +96,29 @@ public AddressEntry(DeterministicKey keyPair, Context context, boolean segwit) { this(keyPair, context, null, segwit); } - public AddressEntry(@NotNull DeterministicKey keyPair, + public AddressEntry(DeterministicKey keyPair, + Context context, + @Nullable String offerId, + boolean segwit) { + this(keyPair, + context, + offerId, + 0, + segwit); + } + + public AddressEntry(DeterministicKey keyPair, Context context, @Nullable String offerId, + long coinLockedInMultiSig, boolean segwit) { + this(keyPair.getPubKey(), + keyPair.getPubKeyHash(), + context, + offerId, + coinLockedInMultiSig, + segwit); this.keyPair = keyPair; - this.context = context; - this.offerId = offerId; - pubKey = keyPair.getPubKey(); - pubKeyHash = keyPair.getPubKeyHash(); - this.segwit = segwit; } @@ -114,13 +130,13 @@ private AddressEntry(byte[] pubKey, byte[] pubKeyHash, Context context, @Nullable String offerId, - Coin coinLockedInMultiSig, + long coinLockedInMultiSig, boolean segwit) { this.pubKey = pubKey; this.pubKeyHash = pubKeyHash; this.context = context; this.offerId = offerId; - this.coinLockedInMultiSig = coinLockedInMultiSig.value; + this.coinLockedInMultiSig = coinLockedInMultiSig; this.segwit = segwit; } @@ -129,7 +145,7 @@ public static AddressEntry fromProto(protobuf.AddressEntry proto) { proto.getPubKeyHash().toByteArray(), ProtoUtil.enumFromProto(AddressEntry.Context.class, proto.getContext().name()), ProtoUtil.stringOrNullFromProto(proto.getOfferId()), - Coin.valueOf(proto.getCoinLockedInMultiSig()), + proto.getCoinLockedInMultiSig(), proto.getSegwit()); } @@ -164,10 +180,6 @@ public DeterministicKey getKeyPair() { return keyPair; } - public void setCoinLockedInMultiSig(@NotNull Coin coinLockedInMultiSig) { - this.coinLockedInMultiSig = coinLockedInMultiSig.value; - } - // For display we usually only display the first 8 characters. @Nullable public String getShortOfferId() { @@ -196,14 +208,14 @@ public boolean isTrade() { return context == Context.MULTI_SIG || context == Context.TRADE_PAYOUT; } - public Coin getCoinLockedInMultiSig() { + public Coin getCoinLockedInMultiSigAsCoin() { return Coin.valueOf(coinLockedInMultiSig); } @Override public String toString() { return "AddressEntry{" + - "address=" + address + + "address=" + getAddress() + ", context=" + context + ", offerId='" + offerId + '\'' + ", coinLockedInMultiSig=" + coinLockedInMultiSig + diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java index ea25be37756..4b41e1c275b 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -196,6 +196,15 @@ public void addAddressEntry(AddressEntry addressEntry) { } public void swapToAvailable(AddressEntry addressEntry) { + if (addressEntry.getContext() == AddressEntry.Context.MULTI_SIG) { + log.error("swapToAvailable called with an addressEntry with MULTI_SIG context. " + + "This in not permitted as we must not reuse those address entries and there are " + + "no redeemable funds on those addresses. " + + "Only the keys are used for creating the Multisig address. " + + "addressEntry={}", addressEntry); + return; + } + boolean setChangedByRemove = entrySet.remove(addressEntry); boolean setChangedByAdd = entrySet.add(new AddressEntry(addressEntry.getKeyPair(), AddressEntry.Context.AVAILABLE, @@ -217,6 +226,24 @@ public AddressEntry swapAvailableToAddressEntryWithOfferId(AddressEntry addressE return newAddressEntry; } + public void setCoinLockedInMultiSigAddressEntry(AddressEntry addressEntry, long value) { + if (addressEntry.getContext() != AddressEntry.Context.MULTI_SIG) { + log.error("setCoinLockedInMultiSigAddressEntry must be called only on MULTI_SIG entries"); + return; + } + + boolean setChangedByRemove = entrySet.remove(addressEntry); + AddressEntry entry = new AddressEntry(addressEntry.getKeyPair(), + addressEntry.getContext(), + addressEntry.getOfferId(), + value, + addressEntry.isSegwit()); + boolean setChangedByAdd = entrySet.add(entry); + if (setChangedByRemove || setChangedByAdd) { + requestPersistence(); + } + } + public void requestPersistence() { persistenceManager.requestPersistence(); } diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index d5726f10f0f..c139c0f642e 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -738,6 +738,14 @@ public List getAddressEntryListAsImmutableList() { } public void swapTradeEntryToAvailableEntry(String offerId, AddressEntry.Context context) { + if (context == AddressEntry.Context.MULTI_SIG) { + log.error("swapTradeEntryToAvailableEntry called with MULTI_SIG context. " + + "This in not permitted as we must not reuse those address entries and there " + + "are no redeemable funds on that addresses. Only the keys are used for creating " + + "the Multisig address. offerId={}, context={}", offerId, context); + return; + } + getAddressEntryListAsImmutableList().stream() .filter(e -> offerId.equals(e.getOfferId())) .filter(e -> context == e.getContext()) @@ -748,6 +756,23 @@ public void swapTradeEntryToAvailableEntry(String offerId, AddressEntry.Context }); } + // When funds from MultiSig address is spent we reset the coinLockedInMultiSig value to 0. + public void resetCoinLockedInMultiSigAddressEntry(String offerId) { + setCoinLockedInMultiSigAddressEntry(offerId, 0); + } + + public void setCoinLockedInMultiSigAddressEntry(String offerId, long value) { + getAddressEntryListAsImmutableList().stream() + .filter(e -> AddressEntry.Context.MULTI_SIG == e.getContext()) + .filter(e -> offerId.equals(e.getOfferId())) + .forEach(addressEntry -> setCoinLockedInMultiSigAddressEntry(addressEntry, value)); + } + + public void setCoinLockedInMultiSigAddressEntry(AddressEntry addressEntry, long value) { + log.info("Set coinLockedInMultiSig for addressEntry {} to value {}", addressEntry, value); + addressEntryList.setCoinLockedInMultiSigAddressEntry(addressEntry, value); + } + public void resetAddressEntriesForOpenOffer(String offerId) { log.info("resetAddressEntriesForOpenOffer offerId={}", offerId); swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.OFFER_FUNDING); @@ -755,8 +780,11 @@ public void resetAddressEntriesForOpenOffer(String offerId) { } public void resetAddressEntriesForPendingTrade(String offerId) { - swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.MULTI_SIG); - // We swap also TRADE_PAYOUT to be sure all is cleaned up. There might be cases where a user cannot send the funds + // We must not swap MULTI_SIG entries as those addresses are not detected in the isAddressUnused + // check at getOrCreateAddressEntry and could lead to a reuse of those keys and result in the same 2of2 MS + // address if same peers trade again. + + // We swap TRADE_PAYOUT to be sure all is cleaned up. There might be cases where a user cannot send the funds // to an external wallet directly in the last step of the trade, but the funds are in the Bisq wallet anyway and // the dealing with the external wallet is pure UI thing. The user can move the funds to the wallet and then // send out the funds to the external wallet. As this cleanup is a rare situation and most users do not use diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/SetupPayoutTxListener.java b/core/src/main/java/bisq/core/trade/protocol/tasks/SetupPayoutTxListener.java index 3c264335535..5d925ae41c2 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/SetupPayoutTxListener.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/SetupPayoutTxListener.java @@ -71,7 +71,7 @@ public void onTransactionConfidenceChanged(TransactionConfidence confidence) { tradeStateSubscription = EasyBind.subscribe(trade.stateProperty(), newValue -> { if (trade.isPayoutPublished()) { - swapMultiSigEntry(); + processModel.getBtcWalletService().resetCoinLockedInMultiSigAddressEntry(trade.getId()); // hack to remove tradeStateSubscription at callback UserThread.execute(this::unSubscribe); @@ -98,16 +98,12 @@ private void applyConfidence(TransactionConfidence confidence) { log.info("We had the payout tx already set. tradeId={}, state={}", trade.getId(), trade.getState()); } - swapMultiSigEntry(); + processModel.getBtcWalletService().resetCoinLockedInMultiSigAddressEntry(trade.getId()); // need delay as it can be called inside the handler before the listener and tradeStateSubscription are actually set. UserThread.execute(this::unSubscribe); } - private void swapMultiSigEntry() { - processModel.getBtcWalletService().swapTradeEntryToAvailableEntry(trade.getId(), AddressEntry.Context.MULTI_SIG); - } - private boolean isInNetwork(TransactionConfidence confidence) { return confidence != null && (confidence.getConfidenceType().equals(TransactionConfidence.ConfidenceType.BUILDING) || diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/arbitration/PublishedDelayedPayoutTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/arbitration/PublishedDelayedPayoutTx.java index 005dd886115..b35e1f53a62 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/arbitration/PublishedDelayedPayoutTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/arbitration/PublishedDelayedPayoutTx.java @@ -18,7 +18,6 @@ package bisq.core.trade.protocol.tasks.arbitration; import bisq.core.btc.exceptions.TxBroadcastException; -import bisq.core.btc.model.AddressEntry; import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.TxBroadcaster; import bisq.core.btc.wallet.WalletService; @@ -46,7 +45,7 @@ protected void run() { BtcWalletService btcWalletService = processModel.getBtcWalletService(); // We have spent the funds from the deposit tx with the delayedPayoutTx - btcWalletService.swapTradeEntryToAvailableEntry(trade.getId(), AddressEntry.Context.MULTI_SIG); + btcWalletService.resetCoinLockedInMultiSigAddressEntry(trade.getId()); // We might receive funds on AddressEntry.Context.TRADE_PAYOUT so we don't swap that Transaction committedDelayedPayoutTx = WalletService.maybeAddSelfTxToWallet(delayedPayoutTx, btcWalletService.getWallet()); 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 71c250b7b91..5b546201afb 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 @@ -18,7 +18,6 @@ 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; import bisq.core.trade.Trade; @@ -60,7 +59,7 @@ protected void run() { BtcWalletService.printTx("payoutTx received from peer", committedPayoutTx); trade.setState(Trade.State.BUYER_RECEIVED_PAYOUT_TX_PUBLISHED_MSG); - processModel.getBtcWalletService().swapTradeEntryToAvailableEntry(trade.getId(), AddressEntry.Context.MULTI_SIG); + processModel.getBtcWalletService().resetCoinLockedInMultiSigAddressEntry(trade.getId()); } else { log.info("We got the payout tx already set from BuyerSetupPayoutTxListener and do nothing here. trade ID={}", trade.getId()); } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java index f78156ef5e0..42b8ff4baa9 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java @@ -28,14 +28,21 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.NetworkParameters; +import org.bitcoinj.core.Sha256Hash; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; +import org.bitcoinj.core.TransactionInput; +import org.bitcoinj.core.TransactionOutPoint; import org.fxmisc.easybind.EasyBind; import org.fxmisc.easybind.Subscription; +import java.util.Objects; + import lombok.extern.slf4j.Slf4j; +import javax.annotation.Nullable; + import static com.google.common.base.Preconditions.checkArgument; @Slf4j @@ -59,14 +66,20 @@ protected void run() { Transaction preparedDepositTx = new Transaction(params, processModel.getPreparedDepositTx()); checkArgument(!preparedDepositTx.getOutputs().isEmpty(), "preparedDepositTx.getOutputs() must not be empty"); Address depositTxAddress = preparedDepositTx.getOutput(0).getScriptPubKey().getToAddress(params); + + // For buyer as maker takerFeeTxId is null + @Nullable String takerFeeTxId = trade.getTakerFeeTxId(); + String makerFeeTxId = trade.getOffer().getOfferFeePaymentTxId(); TransactionConfidence confidence = walletService.getConfidenceForAddress(depositTxAddress); - if (isVisibleInNetwork(confidence)) { + if (isConfTxDepositTx(confidence, params, depositTxAddress, takerFeeTxId, makerFeeTxId) && + isVisibleInNetwork(confidence)) { applyConfidence(confidence); } else { confidenceListener = new AddressConfidenceListener(depositTxAddress) { @Override public void onTransactionConfidenceChanged(TransactionConfidence confidence) { - if (isVisibleInNetwork(confidence)) { + if (isConfTxDepositTx(confidence, params, depositTxAddress, + takerFeeTxId, makerFeeTxId) && isVisibleInNetwork(confidence)) { applyConfidence(confidence); } } @@ -91,6 +104,56 @@ public void onTransactionConfidenceChanged(TransactionConfidence confidence) { } } + // We check if the txIds of the inputs matches our maker fee tx and taker fee tx and if the depositTxAddress we + // use for the confidence lookup is use as an output address. + // This prevents that past txs which have the our depositTxAddress as input or output (deposit or payout txs) could + // be interpreted as our deposit tx. This happened because if a bug which caused re-use of the Multisig address + // entries and if both traders use the same key for multiple trades the depositTxAddress would be the same. + // We fix that bug as well but we also need to avoid that past already used addresses might be taken again + // (the Multisig flag got reverted to available in the address entry). + private boolean isConfTxDepositTx(@Nullable TransactionConfidence confidence, + NetworkParameters params, + Address depositTxAddress, + @Nullable String takerFeeTxId, + String makerFeeTxId) { + if (confidence == null) { + return false; + } + + Transaction walletTx = processModel.getTradeWalletService().getWalletTx(confidence.getTransactionHash()); + long numInputMatches = walletTx.getInputs().stream() + .map(TransactionInput::getOutpoint) + .filter(Objects::nonNull) + .map(TransactionOutPoint::getHash) + .map(Sha256Hash::toString) + .filter(txId -> txId.equals(takerFeeTxId) || txId.equals(makerFeeTxId)) + .count(); + if (takerFeeTxId == null && numInputMatches != 1) { + log.warn("We got a transactionConfidenceTx which does not match our inputs. " + + "takerFeeTxId is null (valid if role is buyer as maker) and numInputMatches " + + "is not 1 as expected (for makerFeeTxId). " + + "numInputMatches={}, transactionConfidenceTx={}", + numInputMatches, walletTx); + return false; + } else if (takerFeeTxId != null && numInputMatches != 2) { + log.warn("We got a transactionConfidenceTx which does not match our inputs. " + + "numInputMatches is not 2 as expected (for makerFeeTxId and takerFeeTxId). " + + "numInputMatches={}, transactionConfidenceTx={}", + numInputMatches, walletTx); + return false; + } + + boolean isOutputMatching = walletTx.getOutputs().stream() + .map(transactionOutput -> transactionOutput.getScriptPubKey().getToAddress(params)) + .anyMatch(address -> address.equals(depositTxAddress)); + if (!isOutputMatching) { + log.warn("We got a transactionConfidenceTx which does not has the depositTxAddress " + + "as output (but as input). depositTxAddress={}, transactionConfidenceTx={}", + depositTxAddress, walletTx); + } + return isOutputMatching; + } + private void applyConfidence(TransactionConfidence confidence) { if (trade.getDepositTx() == null) { Transaction walletTx = processModel.getTradeWalletService().getWalletTx(confidence.getTransactionHash()); diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_maker/BuyerAsMakerCreatesAndSignsDepositTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_maker/BuyerAsMakerCreatesAndSignsDepositTx.java index 4fc92bda031..5932f735d0f 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_maker/BuyerAsMakerCreatesAndSignsDepositTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_maker/BuyerAsMakerCreatesAndSignsDepositTx.java @@ -63,7 +63,7 @@ protected void run() { Optional addressEntryOptional = walletService.getAddressEntry(id, AddressEntry.Context.MULTI_SIG); checkArgument(addressEntryOptional.isPresent(), "addressEntryOptional must be present"); AddressEntry makerMultiSigAddressEntry = addressEntryOptional.get(); - makerMultiSigAddressEntry.setCoinLockedInMultiSig(makerInputAmount); + processModel.getBtcWalletService().setCoinLockedInMultiSigAddressEntry(makerMultiSigAddressEntry, makerInputAmount.value); walletService.saveAddressEntryList(); Coin msOutputAmount = makerInputAmount diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerSignsDepositTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerSignsDepositTx.java index 24f988120da..6833f627314 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerSignsDepositTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerSignsDepositTx.java @@ -68,7 +68,8 @@ protected void run() { AddressEntry buyerMultiSigAddressEntry = addressEntryOptional.get(); Coin buyerInput = Coin.valueOf(buyerInputs.stream().mapToLong(input -> input.value).sum()); - buyerMultiSigAddressEntry.setCoinLockedInMultiSig(buyerInput.subtract(trade.getTxFee().multiply(2))); + Coin multiSigValue = buyerInput.subtract(trade.getTxFee().multiply(2)); + processModel.getBtcWalletService().setCoinLockedInMultiSigAddressEntry(buyerMultiSigAddressEntry, multiSigValue.value); walletService.saveAddressEntryList(); TradingPeer tradingPeer = processModel.getTradingPeer(); diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/FinalizeMediatedPayoutTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/FinalizeMediatedPayoutTx.java index 87a8bdf32e4..875b78f7af4 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/FinalizeMediatedPayoutTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/mediation/FinalizeMediatedPayoutTx.java @@ -110,7 +110,7 @@ protected void run() { processModel.getTradeManager().requestPersistence(); - walletService.swapTradeEntryToAvailableEntry(tradeId, AddressEntry.Context.MULTI_SIG); + walletService.resetCoinLockedInMultiSigAddressEntry(tradeId); complete(); } catch (Throwable t) { 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 8e5b08a068e..39435ab6ec7 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 @@ -17,7 +17,6 @@ package bisq.core.trade.protocol.tasks.mediation; -import bisq.core.btc.model.AddressEntry; import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletService; import bisq.core.support.dispute.mediation.MediationResultState; @@ -70,7 +69,7 @@ protected void run() { .closeDisputedTrade(trade.getId(), Trade.DisputeState.MEDIATION_CLOSED)); } - processModel.getBtcWalletService().swapTradeEntryToAvailableEntry(trade.getId(), AddressEntry.Context.MULTI_SIG); + processModel.getBtcWalletService().resetCoinLockedInMultiSigAddressEntry(trade.getId()); } else { log.info("We got the payout tx already set from BuyerSetupPayoutTxListener and do nothing here. trade ID={}", trade.getId()); } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSignAndFinalizePayoutTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSignAndFinalizePayoutTx.java index 34f645896b2..d6b0babc655 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSignAndFinalizePayoutTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSignAndFinalizePayoutTx.java @@ -102,7 +102,7 @@ protected void run() { processModel.getTradeManager().requestPersistence(); - walletService.swapTradeEntryToAvailableEntry(id, AddressEntry.Context.MULTI_SIG); + walletService.resetCoinLockedInMultiSigAddressEntry(id); complete(); } catch (Throwable t) { diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerCreatesUnsignedDepositTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerCreatesUnsignedDepositTx.java index 160074e9913..b484324d3f3 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerCreatesUnsignedDepositTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerCreatesUnsignedDepositTx.java @@ -70,7 +70,8 @@ protected void run() { Optional addressEntryOptional = walletService.getAddressEntry(id, AddressEntry.Context.MULTI_SIG); checkArgument(addressEntryOptional.isPresent(), "addressEntryOptional must be present"); AddressEntry makerMultiSigAddressEntry = addressEntryOptional.get(); - makerMultiSigAddressEntry.setCoinLockedInMultiSig(makerInputAmount); + processModel.getBtcWalletService().setCoinLockedInMultiSigAddressEntry(makerMultiSigAddressEntry, makerInputAmount.value); + walletService.saveAddressEntryList(); Coin msOutputAmount = makerInputAmount diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_taker/SellerAsTakerSignsDepositTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_taker/SellerAsTakerSignsDepositTx.java index 887cbcf7636..ead81f5c20f 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_taker/SellerAsTakerSignsDepositTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_taker/SellerAsTakerSignsDepositTx.java @@ -66,7 +66,8 @@ protected void run() { Coin sellerInput = Coin.valueOf(sellerInputs.stream().mapToLong(input -> input.value).sum()); Coin totalFee = trade.getTxFee().multiply(2); // Fee for deposit and payout tx - sellerMultiSigAddressEntry.setCoinLockedInMultiSig(sellerInput.subtract(totalFee)); + Coin multiSigValue = sellerInput.subtract(totalFee); + processModel.getBtcWalletService().setCoinLockedInMultiSigAddressEntry(sellerMultiSigAddressEntry, multiSigValue.value); walletService.saveAddressEntryList(); TradingPeer tradingPeer = processModel.getTradingPeer(); diff --git a/desktop/src/main/java/bisq/desktop/main/funds/locked/LockedListItem.java b/desktop/src/main/java/bisq/desktop/main/funds/locked/LockedListItem.java index b9701b45fe9..fcafade8020 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/locked/LockedListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/locked/LockedListItem.java @@ -78,7 +78,7 @@ public void cleanup() { } private void updateBalance() { - balance = addressEntry.getCoinLockedInMultiSig(); + balance = addressEntry.getCoinLockedInMultiSigAsCoin(); balanceLabel.setText(formatter.formatCoin(this.balance)); } From f843b5477bd04e48fe8a43f7ff741a5bf9dec4db Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 11:19:18 -0500 Subject: [PATCH 04/21] Fixes https://github.com/bisq-network/bisq/issues/4864 When seller if offline we resend the CounterCurrencyTransferStartedMessage at startup. That caused the trade state set to BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG and then after the msg was stored in mailbox to BUYER_STORED_IN_MAILBOX_FIAT_PAYMENT_INITIATED_MSG. Those 2 msg trigger diff. UI states which led to the UI glitch that the UI moved to step 2 and then to step 3 which was correct but confusing to the user. Now we only apply BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG is trade state ordinal is smaller avoiding that UI glitch. --- .../buyer/BuyerSendCounterCurrencyTransferStartedMessage.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 54a334c6d46..bfe50a9be18 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 @@ -82,7 +82,9 @@ protected TradeMessage getMessage(String tradeId) { @Override protected void setStateSent() { - trade.setStateIfValidTransitionTo(Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG); + if (trade.getState().ordinal() < Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG.ordinal()) { + trade.setStateIfValidTransitionTo(Trade.State.BUYER_SENT_FIAT_PAYMENT_INITIATED_MSG); + } processModel.getTradeManager().requestPersistence(); } From fa80553890f5cf0f30c8a1e7d9f6da7d6b186715 Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Thu, 3 Dec 2020 10:21:04 -0600 Subject: [PATCH 05/21] add logging of Tx hex --- core/src/main/java/bisq/core/btc/wallet/TxBroadcaster.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/bisq/core/btc/wallet/TxBroadcaster.java b/core/src/main/java/bisq/core/btc/wallet/TxBroadcaster.java index ea17f30e15b..ce5600d55e4 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TxBroadcaster.java +++ b/core/src/main/java/bisq/core/btc/wallet/TxBroadcaster.java @@ -25,6 +25,7 @@ import org.bitcoinj.core.PeerGroup; import org.bitcoinj.core.Transaction; +import org.bitcoinj.core.Utils; import org.bitcoinj.wallet.Wallet; import com.google.common.util.concurrent.FutureCallback; @@ -90,6 +91,7 @@ public static void broadcastTx(Wallet wallet, PeerGroup peerGroup, Transaction l public static void broadcastTx(Wallet wallet, PeerGroup peerGroup, Transaction tx, Callback callback, int timeOut) { Timer timeoutTimer; final String txId = tx.getTxId().toString(); + log.info("Txid: {} hex: {}", txId, Utils.HEX.encode(tx.bitcoinSerialize())); if (!broadcastTimerMap.containsKey(txId)) { timeoutTimer = UserThread.runAfter(() -> { log.warn("Broadcast of tx {} not completed after {} sec.", txId, timeOut); From 1fdc43e4d0315817fae46e6bda07293c629006f1 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 11:53:35 -0500 Subject: [PATCH 06/21] Increase timeout for trade protocol task runners from 30 to 60 sec --- .../java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java | 4 ++-- .../java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java | 6 +++--- .../bisq/core/trade/protocol/SellerAsMakerProtocol.java | 4 ++-- .../bisq/core/trade/protocol/SellerAsTakerProtocol.java | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) 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 311abacc13f..6374469db3a 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java @@ -86,7 +86,7 @@ public void handleTakeOfferRequest(InputsForDepositTxRequest message, errorMessageHandler.handleErrorMessage(errorMessage); handleTaskRunnerFault(message, errorMessage); })) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } @@ -106,7 +106,7 @@ protected void handle(DelayedPayoutTxSignatureRequest message, NodeAddress peer) BuyerSignsDelayedPayoutTx.class, BuyerFinalizesDelayedPayoutTx.class, BuyerSendsDelayedPayoutTxSignatureResponse.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } 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 1572f238ac3..48631776afb 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java @@ -83,7 +83,7 @@ public void onTakeOffer() { CreateTakerFeeTx.class, BuyerAsTakerCreatesDepositTxInputs.class, TakerSendInputsForDepositTxRequest.class) - .withTimeout(30)) + .withTimeout(60)) .run(() -> { processModel.setTempTradingPeerNodeAddress(trade.getTradingPeerNodeAddress()); processModel.getTradeManager().requestPersistence(); @@ -108,7 +108,7 @@ private void handle(InputsForDepositTxResponse message, NodeAddress peer) { BuyerAsTakerSignsDepositTx.class, BuyerSetupDepositTxListener.class, BuyerAsTakerSendsDepositTxMessage.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } @@ -122,7 +122,7 @@ protected void handle(DelayedPayoutTxSignatureRequest message, NodeAddress peer) BuyerSignsDelayedPayoutTx.class, BuyerFinalizesDelayedPayoutTx.class, BuyerSendsDelayedPayoutTxSignatureResponse.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java index 19b3dea65ac..1b632523685 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java @@ -86,7 +86,7 @@ public void handleTakeOfferRequest(InputsForDepositTxRequest message, errorMessageHandler.handleErrorMessage(errorMessage); handleTaskRunnerFault(message, errorMessage); })) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } @@ -106,7 +106,7 @@ protected void handle(DepositTxMessage message, NodeAddress peer) { SellerCreatesDelayedPayoutTx.class, SellerSignsDelayedPayoutTx.class, SellerSendDelayedPayoutTxSignatureRequest.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerAsTakerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerAsTakerProtocol.java index dfcf2d59578..7adf9026c5c 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerAsTakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerAsTakerProtocol.java @@ -78,7 +78,7 @@ public void onTakeOffer() { CreateTakerFeeTx.class, SellerAsTakerCreatesDepositTxInputs.class, TakerSendInputsForDepositTxRequest.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } @@ -101,7 +101,7 @@ private void handle(InputsForDepositTxResponse message, NodeAddress peer) { SellerCreatesDelayedPayoutTx.class, SellerSignsDelayedPayoutTx.class, SellerSendDelayedPayoutTxSignatureRequest.class) - .withTimeout(30)) + .withTimeout(60)) .executeTasks(); } From 2d5fc33ba48e48add4db494558511703785d5d3a Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 14:19:24 -0500 Subject: [PATCH 07/21] Move MakerRemovesOpenOffer to first task to avoid that if take offer fails early that we get another trade with same id at maker in case another use takes the offer afterwards. --- .../java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java | 2 +- .../java/bisq/core/trade/protocol/SellerAsMakerProtocol.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 6374469db3a..c9280d0e206 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java @@ -100,8 +100,8 @@ protected void handle(DelayedPayoutTxSignatureRequest message, NodeAddress peer) .with(message) .from(peer)) .setup(tasks( - BuyerProcessDelayedPayoutTxSignatureRequest.class, MakerRemovesOpenOffer.class, + BuyerProcessDelayedPayoutTxSignatureRequest.class, BuyerVerifiesPreparedDelayedPayoutTx.class, BuyerSignsDelayedPayoutTx.class, BuyerFinalizesDelayedPayoutTx.class, diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java index 1b632523685..0cdaad17c08 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java @@ -100,8 +100,8 @@ protected void handle(DepositTxMessage message, NodeAddress peer) { .with(message) .from(peer)) .setup(tasks( - SellerAsMakerProcessDepositTxMessage.class, MakerRemovesOpenOffer.class, + SellerAsMakerProcessDepositTxMessage.class, SellerAsMakerFinalizesDepositTx.class, SellerCreatesDelayedPayoutTx.class, SellerSignsDelayedPayoutTx.class, From 8f99ca0b6355405eb83d54ea604b4edf61013975 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 14:43:35 -0500 Subject: [PATCH 08/21] Add uid to trade to make sure that look up for process model cannot fail in case of multiple trades with the same offer id. Use uid instead of the weaker offerId as key for the tradeProtocolByTradeId map --- .../bisq/core/trade/BuyerAsMakerTrade.java | 17 +++++++-- .../bisq/core/trade/BuyerAsTakerTrade.java | 17 +++++++-- .../main/java/bisq/core/trade/BuyerTrade.java | 12 ++++--- .../bisq/core/trade/SellerAsMakerTrade.java | 17 +++++++-- .../bisq/core/trade/SellerAsTakerTrade.java | 17 +++++++-- .../java/bisq/core/trade/SellerTrade.java | 12 ++++--- core/src/main/java/bisq/core/trade/Trade.java | 20 ++++++++--- .../java/bisq/core/trade/TradeManager.java | 35 ++++++++++++++----- proto/src/main/proto/pb.proto | 1 + 9 files changed, 115 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/BuyerAsMakerTrade.java b/core/src/main/java/bisq/core/trade/BuyerAsMakerTrade.java index 272edad6dfd..3a0005f8247 100644 --- a/core/src/main/java/bisq/core/trade/BuyerAsMakerTrade.java +++ b/core/src/main/java/bisq/core/trade/BuyerAsMakerTrade.java @@ -24,8 +24,12 @@ import bisq.network.p2p.NodeAddress; +import bisq.common.proto.ProtoUtil; + import org.bitcoinj.core.Coin; +import java.util.UUID; + import lombok.extern.slf4j.Slf4j; import javax.annotation.Nullable; @@ -45,7 +49,8 @@ public BuyerAsMakerTrade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, txFee, takeOfferFee, @@ -54,7 +59,8 @@ public BuyerAsMakerTrade(Offer offer, mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } /////////////////////////////////////////////////////////////////////////////////////////// @@ -74,6 +80,10 @@ public static Tradable fromProto(protobuf.BuyerAsMakerTrade buyerAsMakerTradePro CoreProtoResolver coreProtoResolver) { protobuf.Trade proto = buyerAsMakerTradeProto.getTrade(); ProcessModel processModel = ProcessModel.fromProto(proto.getProcessModel(), coreProtoResolver); + String uid = ProtoUtil.stringOrNullFromProto(proto.getUid()); + if (uid == null) { + uid = UUID.randomUUID().toString(); + } BuyerAsMakerTrade trade = new BuyerAsMakerTrade( Offer.fromProto(proto.getOffer()), Coin.valueOf(proto.getTxFeeAsLong()), @@ -83,7 +93,8 @@ public static Tradable fromProto(protobuf.BuyerAsMakerTrade buyerAsMakerTradePro proto.hasMediatorNodeAddress() ? NodeAddress.fromProto(proto.getMediatorNodeAddress()) : null, proto.hasRefundAgentNodeAddress() ? NodeAddress.fromProto(proto.getRefundAgentNodeAddress()) : null, btcWalletService, - processModel); + processModel, + uid); trade.setTradeAmountAsLong(proto.getTradeAmountAsLong()); trade.setTradePrice(proto.getTradePrice()); diff --git a/core/src/main/java/bisq/core/trade/BuyerAsTakerTrade.java b/core/src/main/java/bisq/core/trade/BuyerAsTakerTrade.java index 46ba45e4240..2ccf0fb3cb9 100644 --- a/core/src/main/java/bisq/core/trade/BuyerAsTakerTrade.java +++ b/core/src/main/java/bisq/core/trade/BuyerAsTakerTrade.java @@ -24,8 +24,12 @@ import bisq.network.p2p.NodeAddress; +import bisq.common.proto.ProtoUtil; + import org.bitcoinj.core.Coin; +import java.util.UUID; + import lombok.extern.slf4j.Slf4j; import javax.annotation.Nullable; @@ -48,7 +52,8 @@ public BuyerAsTakerTrade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, tradeAmount, txFee, @@ -60,7 +65,8 @@ public BuyerAsTakerTrade(Offer offer, mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } @@ -81,6 +87,10 @@ public static Tradable fromProto(protobuf.BuyerAsTakerTrade buyerAsTakerTradePro CoreProtoResolver coreProtoResolver) { protobuf.Trade proto = buyerAsTakerTradeProto.getTrade(); ProcessModel processModel = ProcessModel.fromProto(proto.getProcessModel(), coreProtoResolver); + String uid = ProtoUtil.stringOrNullFromProto(proto.getUid()); + if (uid == null) { + uid = UUID.randomUUID().toString(); + } return fromProto(new BuyerAsTakerTrade( Offer.fromProto(proto.getOffer()), Coin.valueOf(proto.getTradeAmountAsLong()), @@ -93,7 +103,8 @@ public static Tradable fromProto(protobuf.BuyerAsTakerTrade buyerAsTakerTradePro proto.hasMediatorNodeAddress() ? NodeAddress.fromProto(proto.getMediatorNodeAddress()) : null, proto.hasRefundAgentNodeAddress() ? NodeAddress.fromProto(proto.getRefundAgentNodeAddress()) : null, btcWalletService, - processModel), + processModel, + uid), proto, coreProtoResolver); } diff --git a/core/src/main/java/bisq/core/trade/BuyerTrade.java b/core/src/main/java/bisq/core/trade/BuyerTrade.java index f3afd4eb9a9..82f38cf9c16 100644 --- a/core/src/main/java/bisq/core/trade/BuyerTrade.java +++ b/core/src/main/java/bisq/core/trade/BuyerTrade.java @@ -44,7 +44,8 @@ public abstract class BuyerTrade extends Trade { @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, tradeAmount, txFee, @@ -56,7 +57,8 @@ public abstract class BuyerTrade extends Trade { mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } BuyerTrade(Offer offer, @@ -67,7 +69,8 @@ public abstract class BuyerTrade extends Trade { @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, txFee, takerFee, @@ -76,7 +79,8 @@ public abstract class BuyerTrade extends Trade { mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } @Override diff --git a/core/src/main/java/bisq/core/trade/SellerAsMakerTrade.java b/core/src/main/java/bisq/core/trade/SellerAsMakerTrade.java index cd871fd9572..ccbf66a0f6c 100644 --- a/core/src/main/java/bisq/core/trade/SellerAsMakerTrade.java +++ b/core/src/main/java/bisq/core/trade/SellerAsMakerTrade.java @@ -24,8 +24,12 @@ import bisq.network.p2p.NodeAddress; +import bisq.common.proto.ProtoUtil; + import org.bitcoinj.core.Coin; +import java.util.UUID; + import lombok.extern.slf4j.Slf4j; import javax.annotation.Nullable; @@ -45,7 +49,8 @@ public SellerAsMakerTrade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, txFee, takerFee, @@ -54,7 +59,8 @@ public SellerAsMakerTrade(Offer offer, mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } @@ -75,6 +81,10 @@ public static Tradable fromProto(protobuf.SellerAsMakerTrade sellerAsMakerTradeP CoreProtoResolver coreProtoResolver) { protobuf.Trade proto = sellerAsMakerTradeProto.getTrade(); ProcessModel processModel = ProcessModel.fromProto(proto.getProcessModel(), coreProtoResolver); + String uid = ProtoUtil.stringOrNullFromProto(proto.getUid()); + if (uid == null) { + uid = UUID.randomUUID().toString(); + } SellerAsMakerTrade trade = new SellerAsMakerTrade( Offer.fromProto(proto.getOffer()), Coin.valueOf(proto.getTxFeeAsLong()), @@ -84,7 +94,8 @@ public static Tradable fromProto(protobuf.SellerAsMakerTrade sellerAsMakerTradeP proto.hasMediatorNodeAddress() ? NodeAddress.fromProto(proto.getMediatorNodeAddress()) : null, proto.hasRefundAgentNodeAddress() ? NodeAddress.fromProto(proto.getRefundAgentNodeAddress()) : null, btcWalletService, - processModel); + processModel, + uid); trade.setTradeAmountAsLong(proto.getTradeAmountAsLong()); trade.setTradePrice(proto.getTradePrice()); diff --git a/core/src/main/java/bisq/core/trade/SellerAsTakerTrade.java b/core/src/main/java/bisq/core/trade/SellerAsTakerTrade.java index 5fb88bf479d..11fb6c281d0 100644 --- a/core/src/main/java/bisq/core/trade/SellerAsTakerTrade.java +++ b/core/src/main/java/bisq/core/trade/SellerAsTakerTrade.java @@ -24,8 +24,12 @@ import bisq.network.p2p.NodeAddress; +import bisq.common.proto.ProtoUtil; + import org.bitcoinj.core.Coin; +import java.util.UUID; + import lombok.extern.slf4j.Slf4j; import javax.annotation.Nullable; @@ -48,7 +52,8 @@ public SellerAsTakerTrade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, tradeAmount, txFee, @@ -60,7 +65,8 @@ public SellerAsTakerTrade(Offer offer, mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } @@ -81,6 +87,10 @@ public static Tradable fromProto(protobuf.SellerAsTakerTrade sellerAsTakerTradeP CoreProtoResolver coreProtoResolver) { protobuf.Trade proto = sellerAsTakerTradeProto.getTrade(); ProcessModel processModel = ProcessModel.fromProto(proto.getProcessModel(), coreProtoResolver); + String uid = ProtoUtil.stringOrNullFromProto(proto.getUid()); + if (uid == null) { + uid = UUID.randomUUID().toString(); + } return fromProto(new SellerAsTakerTrade( Offer.fromProto(proto.getOffer()), Coin.valueOf(proto.getTradeAmountAsLong()), @@ -93,7 +103,8 @@ public static Tradable fromProto(protobuf.SellerAsTakerTrade sellerAsTakerTradeP proto.hasMediatorNodeAddress() ? NodeAddress.fromProto(proto.getMediatorNodeAddress()) : null, proto.hasRefundAgentNodeAddress() ? NodeAddress.fromProto(proto.getRefundAgentNodeAddress()) : null, btcWalletService, - processModel), + processModel, + uid), proto, coreProtoResolver); } diff --git a/core/src/main/java/bisq/core/trade/SellerTrade.java b/core/src/main/java/bisq/core/trade/SellerTrade.java index 353ff62bd5d..a87c18ddee9 100644 --- a/core/src/main/java/bisq/core/trade/SellerTrade.java +++ b/core/src/main/java/bisq/core/trade/SellerTrade.java @@ -45,7 +45,8 @@ public abstract class SellerTrade extends Trade { @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, tradeAmount, txFee, @@ -57,7 +58,8 @@ public abstract class SellerTrade extends Trade { mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } SellerTrade(Offer offer, @@ -68,7 +70,8 @@ public abstract class SellerTrade extends Trade { @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { super(offer, txFee, takeOfferFee, @@ -77,7 +80,8 @@ public abstract class SellerTrade extends Trade { mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); } @Override diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index ba503622629..19ae07f99a4 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -296,6 +296,11 @@ public static protobuf.Trade.TradePeriodState toProtoMessage(Trade.TradePeriodSt private final long txFeeAsLong; @Getter private final long takerFeeAsLong; + + // Added in 1.5.1 + @Getter + private final String uid; + @Setter private long takeOfferDate; @@ -470,7 +475,8 @@ protected Trade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { this.offer = offer; this.txFee = txFee; this.takerFee = takerFee; @@ -480,10 +486,13 @@ protected Trade(Offer offer, this.refundAgentNodeAddress = refundAgentNodeAddress; this.btcWalletService = btcWalletService; this.processModel = processModel; + this.uid = uid; txFeeAsLong = txFee.value; takerFeeAsLong = takerFee.value; takeOfferDate = new Date().getTime(); + + log.error("New trade created with offerId={} and Uid={}", offer.getId(), uid); } @@ -500,7 +509,8 @@ protected Trade(Offer offer, @Nullable NodeAddress mediatorNodeAddress, @Nullable NodeAddress refundAgentNodeAddress, BtcWalletService btcWalletService, - ProcessModel processModel) { + ProcessModel processModel, + String uid) { this(offer, txFee, @@ -510,7 +520,8 @@ protected Trade(Offer offer, mediatorNodeAddress, refundAgentNodeAddress, btcWalletService, - processModel); + processModel, + uid); this.tradePrice = tradePrice; this.tradingPeerNodeAddress = tradingPeerNodeAddress; @@ -539,7 +550,8 @@ public Message toProtoMessage() { .addAllChatMessage(chatMessages.stream() .map(msg -> msg.toProtoNetworkEnvelope().getChatMessage()) .collect(Collectors.toList())) - .setLockTime(lockTime); + .setLockTime(lockTime) + .setUid(uid); Optional.ofNullable(takerFeeTxId).ifPresent(builder::setTakerFeeTxId); Optional.ofNullable(depositTxId).ifPresent(builder::setDepositTxId); diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index 5bbd88c4cd5..a2d142c8e07 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -89,6 +89,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -259,7 +260,8 @@ private void handleTakeOfferRequest(NodeAddress peer, InputsForDepositTxRequest openOffer.getMediatorNodeAddress(), openOffer.getRefundAgentNodeAddress(), btcWalletService, - getNewProcessModel(offer)); + getNewProcessModel(offer), + UUID.randomUUID().toString()); } else { trade = new SellerAsMakerTrade(offer, Coin.valueOf(inputsForDepositTxRequest.getTxFee()), @@ -269,10 +271,15 @@ private void handleTakeOfferRequest(NodeAddress peer, InputsForDepositTxRequest openOffer.getMediatorNodeAddress(), openOffer.getRefundAgentNodeAddress(), btcWalletService, - getNewProcessModel(offer)); + getNewProcessModel(offer), + UUID.randomUUID().toString()); } TradeProtocol tradeProtocol = TradeProtocolFactory.getNewTradeProtocol(trade); - tradeProtocolByTradeId.put(trade.getId(), tradeProtocol); + TradeProtocol prev = tradeProtocolByTradeId.put(trade.getUid(), tradeProtocol); + if (prev != null) { + log.error("We had already an entry with uid {}", trade.getUid()); + } + tradableList.add(trade); initTradeAndProtocol(trade, tradeProtocol); @@ -313,11 +320,16 @@ public void onUpdatedDataReceived() { } public TradeProtocol getTradeProtocol(Trade trade) { - if (tradeProtocolByTradeId.containsKey(trade.getId())) { - return tradeProtocolByTradeId.get(trade.getId()); + String uid = trade.getUid(); + if (tradeProtocolByTradeId.containsKey(uid)) { + return tradeProtocolByTradeId.get(uid); } else { TradeProtocol tradeProtocol = TradeProtocolFactory.getNewTradeProtocol(trade); - tradeProtocolByTradeId.put(trade.getId(), tradeProtocol); + TradeProtocol prev = tradeProtocolByTradeId.put(uid, tradeProtocol); + if (prev != null) { + log.error("We had already an entry with uid {}", trade.getUid()); + } + return tradeProtocol; } } @@ -406,7 +418,8 @@ public void onTakeOffer(Coin amount, model.getSelectedMediator(), model.getSelectedRefundAgent(), btcWalletService, - getNewProcessModel(offer)); + getNewProcessModel(offer), + UUID.randomUUID().toString()); } else { trade = new BuyerAsTakerTrade(offer, amount, @@ -419,14 +432,18 @@ public void onTakeOffer(Coin amount, model.getSelectedMediator(), model.getSelectedRefundAgent(), btcWalletService, - getNewProcessModel(offer)); + getNewProcessModel(offer), + UUID.randomUUID().toString()); } trade.getProcessModel().setUseSavingsWallet(useSavingsWallet); trade.getProcessModel().setFundsNeededForTradeAsLong(fundsNeededForTrade.value); trade.setTakerPaymentAccountId(paymentAccountId); TradeProtocol tradeProtocol = TradeProtocolFactory.getNewTradeProtocol(trade); - tradeProtocolByTradeId.put(trade.getId(), tradeProtocol); + TradeProtocol prev = tradeProtocolByTradeId.put(trade.getUid(), tradeProtocol); + if (prev != null) { + log.error("We had already an entry with uid {}", trade.getUid()); + } tradableList.add(trade); initTradeAndProtocol(trade, tradeProtocol); diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 63dea6c6fe9..22d81d40de9 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -1449,6 +1449,7 @@ message Trade { int64 last_refresh_request_date = 36 [deprecated = true]; string counter_currency_extra_data = 37; string asset_tx_proof_result = 38; // name of AssetTxProofResult enum + string uid = 39; } message BuyerAsMakerTrade { From 546c7546ac1f3106726e0c475dd73fe89ed534c4 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Dec 2020 14:57:33 -0500 Subject: [PATCH 09/21] Increase max number of log files to 20. Requested here: https://github.com/bisq-network/bisq/issues/4856 --- common/src/main/java/bisq/common/app/Log.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/bisq/common/app/Log.java b/common/src/main/java/bisq/common/app/Log.java index ed098d32c9b..a594292289c 100644 --- a/common/src/main/java/bisq/common/app/Log.java +++ b/common/src/main/java/bisq/common/app/Log.java @@ -48,7 +48,7 @@ public static void setup(String fileName) { rollingPolicy.setParent(appender); rollingPolicy.setFileNamePattern(fileName + "_%i.log"); rollingPolicy.setMinIndex(1); - rollingPolicy.setMaxIndex(10); + rollingPolicy.setMaxIndex(20); rollingPolicy.start(); SizeBasedTriggeringPolicy triggeringPolicy = new SizeBasedTriggeringPolicy<>(); From c04b95b99644b46aae29bf22deeab017c72a5e23 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 4 Dec 2020 10:10:57 -0500 Subject: [PATCH 10/21] Remove checkNotNull for takerFeeTxId --- .../core/trade/protocol/tasks/maker/MakerRemovesOpenOffer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerRemovesOpenOffer.java b/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerRemovesOpenOffer.java index 9c0ae01ad78..6fe8c587698 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerRemovesOpenOffer.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerRemovesOpenOffer.java @@ -37,8 +37,6 @@ protected void run() { try { runInterceptHook(); - // Once the taker fee is published we remove our open offer - checkNotNull(trade.getTakerFeeTxId()); processModel.getOpenOfferManager().closeOpenOffer(checkNotNull(trade.getOffer())); complete(); From 9ca20d8b3da07286061b85a2b3dd79b6076a1723 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Sun, 6 Dec 2020 19:43:12 -0300 Subject: [PATCH 11/21] Use bitcoinj 0.15.8 (commit dcf8af0) --- build.gradle | 2 +- core/src/main/java/bisq/core/app/WalletAppSetup.java | 2 +- gradle/witness/gradle-witness.gradle | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index f40319ce85e..95149e9cd9e 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ configure(subprojects) { ext { // in alphabetical order bcVersion = '1.63' - bitcoinjVersion = '7752cb7' + bitcoinjVersion = 'dcf8af0' btcdCli4jVersion = '27b94333' codecVersion = '1.13' easybindVersion = '1.0.3' diff --git a/core/src/main/java/bisq/core/app/WalletAppSetup.java b/core/src/main/java/bisq/core/app/WalletAppSetup.java index 8d844561b3e..edb3cd7381d 100644 --- a/core/src/main/java/bisq/core/app/WalletAppSetup.java +++ b/core/src/main/java/bisq/core/app/WalletAppSetup.java @@ -107,7 +107,7 @@ void init(@Nullable Consumer chainFileLockedExceptionHandler, Runnable downloadCompleteHandler, Runnable walletInitializedHandler) { log.info("Initialize WalletAppSetup with BitcoinJ version {} and hash of BitcoinJ commit {}", - VersionMessage.BITCOINJ_VERSION, "7752cb7"); + VersionMessage.BITCOINJ_VERSION, "dcf8af0"); ObjectProperty walletServiceException = new SimpleObjectProperty<>(); btcInfoBinding = EasyBind.combine(walletsSetup.downloadPercentageProperty(), diff --git a/gradle/witness/gradle-witness.gradle b/gradle/witness/gradle-witness.gradle index 1ea70a24745..0a49ef0065f 100644 --- a/gradle/witness/gradle-witness.gradle +++ b/gradle/witness/gradle-witness.gradle @@ -23,7 +23,7 @@ dependencyVerification { 'com.github.bisq-network.netlayer:tor.external:a3606a592d6b6caa6a2fb7db224eaf43c6874c6730da4815bd37ad686b283dcb', 'com.github.bisq-network.netlayer:tor.native:b15aba7fe987185037791c7ec7c529cb001b90d723d047d54aab87aceb3b3d45', 'com.github.bisq-network.netlayer:tor:a974190aa3a031067ccd1dda28a3ae58cad14060792299d86ea38a05fb21afc5', - 'com.github.bisq-network:bitcoinj:2d261e53cd0cb6321119e5a48eb4ee2a852b4b0bbbea9dff837a785f05541bb3', + 'com.github.bisq-network:bitcoinj:f6dd3c2c53f61d7f6aed49cfdcd012b4b9db882efd5e5813a45fc23b026c8d40', 'com.github.cd2357.tor-binary:tor-binary-geoip:ae27b6aca1a3a50a046eb11e38202b6d21c2fcd2b8643bbeb5ea85e065fbc1be', 'com.github.cd2357.tor-binary:tor-binary-linux32:7b5d6770aa442ef6d235e8a9bfbaa7c62560690f9fe69ff03c7a752eae84f7dc', 'com.github.cd2357.tor-binary:tor-binary-linux64:24111fa35027599a750b0176392dc1e9417d919414396d1b221ac2e707eaba76', From e2f9009fce42791427abb8ab52f6d3a7643285f3 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 10:33:41 -0500 Subject: [PATCH 12/21] Remove dev log, fix typo in log --- core/src/main/java/bisq/core/trade/Trade.java | 2 -- .../protocol/tasks/buyer/BuyerSetupDepositTxListener.java | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index 19ae07f99a4..f6f7ecd0e2c 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -491,8 +491,6 @@ protected Trade(Offer offer, txFeeAsLong = txFee.value; takerFeeAsLong = takerFee.value; takeOfferDate = new Date().getTime(); - - log.error("New trade created with offerId={} and Uid={}", offer.getId(), uid); } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java index 42b8ff4baa9..9c6bfa2b744 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSetupDepositTxListener.java @@ -129,14 +129,14 @@ private boolean isConfTxDepositTx(@Nullable TransactionConfidence confidence, .filter(txId -> txId.equals(takerFeeTxId) || txId.equals(makerFeeTxId)) .count(); if (takerFeeTxId == null && numInputMatches != 1) { - log.warn("We got a transactionConfidenceTx which does not match our inputs. " + + log.warn("We got a transactionConfidenceTx which does not match our inputs. " + "takerFeeTxId is null (valid if role is buyer as maker) and numInputMatches " + "is not 1 as expected (for makerFeeTxId). " + "numInputMatches={}, transactionConfidenceTx={}", numInputMatches, walletTx); return false; } else if (takerFeeTxId != null && numInputMatches != 2) { - log.warn("We got a transactionConfidenceTx which does not match our inputs. " + + log.warn("We got a transactionConfidenceTx which does not match our inputs. " + "numInputMatches is not 2 as expected (for makerFeeTxId and takerFeeTxId). " + "numInputMatches={}, transactionConfidenceTx={}", numInputMatches, walletTx); From 30ff8c3853aa26173a466c6e910c45983cd823c7 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 14:53:45 -0500 Subject: [PATCH 13/21] Remove paymentAccount from log --- core/src/main/java/bisq/core/offer/CreateOfferService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/bisq/core/offer/CreateOfferService.java b/core/src/main/java/bisq/core/offer/CreateOfferService.java index f4adaa7802f..51a3847eb9e 100644 --- a/core/src/main/java/bisq/core/offer/CreateOfferService.java +++ b/core/src/main/java/bisq/core/offer/CreateOfferService.java @@ -119,9 +119,8 @@ public Offer createAndGetOffer(String offerId, "amount={}, \n" + "minAmount={}, \n" + "buyerSecurityDeposit={}, \n" + - "paymentAccount={}, \n", - offerId, currencyCode, direction, price.getValue(), useMarketBasedPrice, marketPriceMargin, - amount.value, minAmount.value, buyerSecurityDepositAsDouble, paymentAccount); + offerId, currencyCode, direction, price.getValue(), useMarketBasedPrice, marketPriceMargin, + amount.value, minAmount.value, buyerSecurityDepositAsDouble); // prints our param list for dev testing api log.info("{} " + From f4fd286b861a1059869f77f43533226d02e8fc2b Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 17:56:28 -0500 Subject: [PATCH 14/21] Remove verbose log --- core/src/main/java/bisq/core/trade/Trade.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index 19ae07f99a4..9970f391be3 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -888,7 +888,6 @@ private long getTradeStartTime() { startTime = now; } } else { - log.warn("Cannot set TradeStartTime because depositTx is null. TradeId={}", getId()); startTime = now; } return startTime; From 38d52ff6f9b875abe23caec0ac8b5736aebcf02b Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 19:32:32 -0500 Subject: [PATCH 15/21] Add more logs to AddressEntry domain --- .../main/java/bisq/core/btc/model/AddressEntry.java | 6 +++++- .../java/bisq/core/btc/model/AddressEntryList.java | 12 +++++++++--- .../java/bisq/core/btc/wallet/BtcWalletService.java | 2 ++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntry.java b/core/src/main/java/bisq/core/btc/model/AddressEntry.java index c8804fa29a1..fe73686d9ca 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -195,8 +195,12 @@ public String getAddressString() { @Nullable public Address getAddress() { - if (address == null && keyPair != null) + if (address == null && keyPair != null) { address = Address.fromKey(Config.baseCurrencyNetworkParameters(), keyPair, segwit ? Script.ScriptType.P2WPKH : Script.ScriptType.P2PKH); + } + if (address == null) { + log.warn("Address is null at getAddress(). keyPair={}", keyPair); + } return address; } diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java index 4b41e1c275b..3453ab43dce 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -149,11 +149,13 @@ public void onWalletReady(Wallet wallet) { wallet.getIssuedReceiveAddresses().stream() .filter(this::isAddressNotInEntries) .forEach(address -> { - log.info("Create AddressEntry for IssuedReceiveAddress. address={}", address.toString()); - DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(address); + DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(address); if (key != null) { // Address will be derived from key in getAddress method + log.info("Create AddressEntry for IssuedReceiveAddress. address={}", address.toString()); entrySet.add(new AddressEntry(key, AddressEntry.Context.AVAILABLE, address instanceof SegwitAddress)); + } else { + log.warn("DeterministicKey for address {} is null", address); } }); } @@ -190,6 +192,7 @@ public void addAddressEntry(AddressEntry addressEntry) { return; } + log.info("addAddressEntry: add new AddressEntry {}", addressEntry); boolean setChangedByAdd = entrySet.add(addressEntry); if (setChangedByAdd) requestPersistence(); @@ -205,6 +208,7 @@ public void swapToAvailable(AddressEntry addressEntry) { return; } + log.info("swapToAvailable addressEntry to swap={}", addressEntry); boolean setChangedByRemove = entrySet.remove(addressEntry); boolean setChangedByAdd = entrySet.add(new AddressEntry(addressEntry.getKeyPair(), AddressEntry.Context.AVAILABLE, @@ -218,7 +222,8 @@ public AddressEntry swapAvailableToAddressEntryWithOfferId(AddressEntry addressE AddressEntry.Context context, String offerId) { boolean setChangedByRemove = entrySet.remove(addressEntry); - final AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId, addressEntry.isSegwit()); + AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId, addressEntry.isSegwit()); + log.info("swapAvailableToAddressEntryWithOfferId newAddressEntry={}", newAddressEntry); boolean setChangedByAdd = entrySet.add(newAddressEntry); if (setChangedByRemove || setChangedByAdd) requestPersistence(); @@ -232,6 +237,7 @@ public void setCoinLockedInMultiSigAddressEntry(AddressEntry addressEntry, long return; } + log.info("setCoinLockedInMultiSigAddressEntry addressEntry={}, value={}", addressEntry, value); boolean setChangedByRemove = entrySet.remove(addressEntry); AddressEntry entry = new AddressEntry(addressEntry.getKeyPair(), addressEntry.getContext(), diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index c139c0f642e..05aa317e17b 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -638,6 +638,7 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context } else { DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2WPKH)); AddressEntry entry = new AddressEntry(key, context, offerId, true); + log.info("getOrCreateAddressEntry: new AddressEntry={}", entry); addressEntryList.addAddressEntry(entry); return entry; } @@ -689,6 +690,7 @@ private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); } AddressEntry entry = new AddressEntry(key, context, segwit); + log.info("getOrCreateAddressEntry: add new AddressEntry {}", entry); addressEntryList.addAddressEntry(entry); return entry; } From b5e19312c5f104cb16eb74c8119a9f1527ba834a Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 19:35:01 -0500 Subject: [PATCH 16/21] Remove logs logging absolute path to data directory --- .../java/bisq/common/setup/CommonSetup.java | 1 - .../bisq/core/btc/setup/WalletConfig.java | 57 ++++++++++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/common/src/main/java/bisq/common/setup/CommonSetup.java b/common/src/main/java/bisq/common/setup/CommonSetup.java index 5562787ff78..e59c011ebd0 100644 --- a/common/src/main/java/bisq/common/setup/CommonSetup.java +++ b/common/src/main/java/bisq/common/setup/CommonSetup.java @@ -90,7 +90,6 @@ public static void setupUncaughtExceptionHandler(UncaughtExceptionHandler uncaug private static void setupLog(Config config) { String logPath = Paths.get(config.appDataDir.getPath(), "bisq").toString(); Log.setup(logPath); - log.info("Log files under: {}", logPath); Utilities.printSysInfo(); Log.setLevel(Level.toLevel(config.logLevel)); } diff --git a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java index 9f574036429..e90a6601435 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -24,36 +24,66 @@ import bisq.common.config.Config; import bisq.common.file.FileUtil; -import com.google.common.io.Closeables; -import com.google.common.util.concurrent.*; -import org.bitcoinj.core.listeners.*; -import org.bitcoinj.core.*; +import org.bitcoinj.core.BlockChain; +import org.bitcoinj.core.CheckpointManager; +import org.bitcoinj.core.Context; +import org.bitcoinj.core.NetworkParameters; +import org.bitcoinj.core.PeerAddress; +import org.bitcoinj.core.PeerGroup; +import org.bitcoinj.core.listeners.DownloadProgressTracker; import org.bitcoinj.crypto.KeyCrypter; import org.bitcoinj.net.BlockingClientManager; -import org.bitcoinj.net.discovery.*; +import org.bitcoinj.net.discovery.DnsDiscovery; +import org.bitcoinj.net.discovery.PeerDiscovery; import org.bitcoinj.script.Script; -import org.bitcoinj.store.*; -import org.bitcoinj.wallet.*; +import org.bitcoinj.store.BlockStore; +import org.bitcoinj.store.BlockStoreException; +import org.bitcoinj.store.SPVBlockStore; +import org.bitcoinj.wallet.DeterministicKeyChain; +import org.bitcoinj.wallet.DeterministicSeed; +import org.bitcoinj.wallet.KeyChainGroup; +import org.bitcoinj.wallet.KeyChainGroupStructure; +import org.bitcoinj.wallet.Protos; +import org.bitcoinj.wallet.Wallet; +import org.bitcoinj.wallet.WalletExtension; +import org.bitcoinj.wallet.WalletProtobufSerializer; import com.runjva.sourceforge.jsocks.protocol.Socks5Proxy; +import com.google.common.io.Closeables; +import com.google.common.util.concurrent.AbstractIdleService; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; + import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import org.bouncycastle.crypto.params.KeyParameter; -import org.slf4j.*; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.Proxy; -import javax.annotation.*; -import java.io.*; -import java.net.*; -import java.util.concurrent.*; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; + +import java.util.concurrent.TimeUnit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import lombok.Getter; import lombok.Setter; +import javax.annotation.Nullable; + import static bisq.common.util.Preconditions.checkDir; -import static com.google.common.base.Preconditions.*; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; /** *

Utility class that wraps the boilerplate needed to set up a new SPV bitcoinj app. Instantiate it with a directory @@ -233,7 +263,6 @@ protected void onSetupCompleted() { protected void startUp() throws Exception { // Runs in a separate thread. Context.propagate(context); - log.info("Starting up with directory = {}", directory); try { File chainFile = new File(directory, filePrefix + ".spvchain"); boolean chainFileExists = chainFile.exists(); From a3df372ecd6df226ba70a9c78a377d9e48ee9fa9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 19:47:05 -0500 Subject: [PATCH 17/21] Apply project format rules (no code change) --- .../java/bisq/core/btc/wallet/TradeWalletService.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java index 2b05f47c182..aa65a706693 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -735,7 +735,7 @@ public byte[] signDelayedPayoutTx(Transaction delayedPayoutTx, Sha256Hash sigHash; Coin delayedPayoutTxInputValue = preparedDepositTx.getOutput(0).getValue(); sigHash = delayedPayoutTx.hashForWitnessSignature(0, redeemScript, - delayedPayoutTxInputValue, Transaction.SigHash.ALL, false); + delayedPayoutTxInputValue, Transaction.SigHash.ALL, false); checkNotNull(myMultiSigKeyPair, "myMultiSigKeyPair must not be null"); if (myMultiSigKeyPair.isEncrypted()) { checkNotNull(aesKey); @@ -1065,8 +1065,8 @@ public Transaction traderSignAndFinalizeDisputedPayoutTx(byte[] depositTxSeriali // Take care of order of signatures. See comment below at getMultiSigRedeemScript (sort order needed here: arbitrator, seller, buyer) if (hashedMultiSigOutputIsLegacy) { Script inputScript = ScriptBuilder.createP2SHMultiSigInputScript( - ImmutableList.of(arbitratorTxSig, tradersTxSig), - redeemScript); + ImmutableList.of(arbitratorTxSig, tradersTxSig), + redeemScript); input.setScriptSig(inputScript); } else { input.setScriptSig(ScriptBuilder.createEmpty()); @@ -1104,7 +1104,7 @@ public void emergencySignAndPublishPayoutTxFrom2of2MultiSig(String depositTxHex, byte[] sellerPubKey = ECKey.fromPublicOnly(Utils.HEX.decode(sellerPubKeyAsHex)).getPubKey(); Script hashedMultiSigOutputScript = get2of2MultiSigOutputScript(buyerPubKey, sellerPubKey, - hashedMultiSigOutputIsLegacy); + hashedMultiSigOutputIsLegacy); Coin msOutputValue = buyerPayoutAmount.add(sellerPayoutAmount).add(txFee); TransactionOutput hashedMultiSigOutput = new TransactionOutput(params, null, msOutputValue, hashedMultiSigOutputScript.getProgram()); @@ -1147,7 +1147,7 @@ public void emergencySignAndPublishPayoutTxFrom2of2MultiSig(String depositTxHex, TransactionInput input = payoutTx.getInput(0); if (hashedMultiSigOutputIsLegacy) { Script inputScript = ScriptBuilder.createP2SHMultiSigInputScript(ImmutableList.of(sellerTxSig, buyerTxSig), - redeemScript); + redeemScript); input.setScriptSig(inputScript); } else { input.setScriptSig(ScriptBuilder.createEmpty()); From d0005e45f44d23916065263b999a700e622d754f Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 19:47:44 -0500 Subject: [PATCH 18/21] Add logs of tx in case of exceptions --- .../core/btc/wallet/BsqWalletService.java | 3 +- .../core/btc/wallet/TradeWalletService.java | 160 +++++++++--------- 2 files changed, 84 insertions(+), 79 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index 1c13e4c3102..c6a61880006 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -45,12 +45,12 @@ import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.core.Sha256Hash; -import org.bitcoinj.script.ScriptException; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; +import org.bitcoinj.script.ScriptException; import org.bitcoinj.wallet.CoinSelection; import org.bitcoinj.wallet.CoinSelector; import org.bitcoinj.wallet.SendRequest; @@ -565,6 +565,7 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun return tx; } catch (InsufficientMoneyException e) { + log.error("getPreparedSendTx: tx={}", tx.toString()); log.error(e.toString()); throw new InsufficientBsqException(e.missing); } diff --git a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java index aa65a706693..adf0012965d 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -194,10 +194,9 @@ public Transaction createBtcTradingFeeTx(Address fundingAddress, return tradingFeeTx; } catch (Throwable t) { if (wallet != null && sendRequest != null && sendRequest.coinSelector != null) { - log.warn("Balance = {}; CoinSelector = {}", wallet.getBalance(sendRequest.coinSelector), sendRequest.coinSelector); + log.error("Balance = {}; CoinSelector = {}", wallet.getBalance(sendRequest.coinSelector), sendRequest.coinSelector); } - - log.warn("createBtcTradingFeeTx failed: tradingFeeTx={}, txOutputs={}", tradingFeeTx.toString(), + log.error("createBtcTradingFeeTx failed: tradingFeeTx={}, txOutputs={}", tradingFeeTx.toString(), tradingFeeTx.getOutputs()); throw t; } @@ -211,83 +210,88 @@ public Transaction completeBsqTradingFeeTx(Transaction preparedBsqTx, boolean useSavingsWallet, Coin txFee) throws TransactionVerificationException, WalletException, InsufficientMoneyException, AddressFormatException { - // preparedBsqTx has following structure: - // inputs [1-n] BSQ inputs - // outputs [0-1] BSQ change output - // mining fee: burned BSQ fee - - // We add BTC mining fee. Result tx looks like: - // inputs [1-n] BSQ inputs - // inputs [1-n] BTC inputs - // outputs [0-1] BSQ change output - // outputs [1] BTC reservedForTrade output - // outputs [0-1] BTC change output - // mining fee: BTC mining fee + burned BSQ fee - - // In case all BSQ were burnt as fees we have no receiver output and it might be that there are no change outputs - // We need to guarantee that min. 1 valid output is added (OP_RETURN does not count). So we use a higher input - // for BTC to force an additional change output. - - final int preparedBsqTxInputsSize = preparedBsqTx.getInputs().size(); - final boolean hasBsqOutputs = !preparedBsqTx.getOutputs().isEmpty(); - - // If there are no BSQ change outputs an output larger than the burnt BSQ amount has to be added as the first - // output to make sure the reserved funds are in output 1, deposit tx input creation depends on the reserve - // being output 1. The amount has to be larger than the BSQ input to make sure the inputs get burnt. - // The BTC changeAddress is used, so it might get used for both output 0 and output 2. - if (!hasBsqOutputs) { - var bsqInputValue = preparedBsqTx.getInputs().stream() - .map(TransactionInput::getValue) - .reduce(Coin.valueOf(0), Coin::add); - - preparedBsqTx.addOutput(bsqInputValue.add(Coin.valueOf(1)), changeAddress); - } - // the reserved amount we need for the trade we send to our trade reservedForTradeAddress - preparedBsqTx.addOutput(reservedFundsForOffer, reservedForTradeAddress); - - // we allow spending of unconfirmed tx (double spend risk is low and usability would suffer if we need to - // wait for 1 confirmation) - // In case of double spend we will detect later in the trade process and use a ban score to penalize bad behaviour (not impl. yet) - - SendRequest sendRequest = SendRequest.forTx(preparedBsqTx); - sendRequest.shuffleOutputs = false; - sendRequest.aesKey = aesKey; - if (useSavingsWallet) { - sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE), - preferences.getIgnoreDustThreshold()); - } else { - sendRequest.coinSelector = new BtcCoinSelector(fundingAddress, preferences.getIgnoreDustThreshold()); - } - // We use a fixed fee - sendRequest.fee = txFee; - sendRequest.feePerKb = Coin.ZERO; - sendRequest.ensureMinRequiredFee = false; - - sendRequest.signInputs = false; - - // Change is optional in case of overpay or use of funds from savings wallet - sendRequest.changeAddress = changeAddress; - - checkNotNull(wallet, "Wallet must not be null"); - wallet.completeTx(sendRequest); - Transaction resultTx = sendRequest.tx; - removeDust(resultTx); - - // Sign all BTC inputs - for (int i = preparedBsqTxInputsSize; i < resultTx.getInputs().size(); i++) { - TransactionInput txIn = resultTx.getInputs().get(i); - checkArgument(txIn.getConnectedOutput() != null && - txIn.getConnectedOutput().isMine(wallet), - "txIn.getConnectedOutput() is not in our wallet. That must not happen."); - WalletService.signTransactionInput(wallet, aesKey, resultTx, txIn, i); - WalletService.checkScriptSig(resultTx, txIn, i); - } + try { + // preparedBsqTx has following structure: + // inputs [1-n] BSQ inputs + // outputs [0-1] BSQ change output + // mining fee: burned BSQ fee + + // We add BTC mining fee. Result tx looks like: + // inputs [1-n] BSQ inputs + // inputs [1-n] BTC inputs + // outputs [0-1] BSQ change output + // outputs [1] BTC reservedForTrade output + // outputs [0-1] BTC change output + // mining fee: BTC mining fee + burned BSQ fee + + // In case all BSQ were burnt as fees we have no receiver output and it might be that there are no change outputs + // We need to guarantee that min. 1 valid output is added (OP_RETURN does not count). So we use a higher input + // for BTC to force an additional change output. + + final int preparedBsqTxInputsSize = preparedBsqTx.getInputs().size(); + final boolean hasBsqOutputs = !preparedBsqTx.getOutputs().isEmpty(); + + // If there are no BSQ change outputs an output larger than the burnt BSQ amount has to be added as the first + // output to make sure the reserved funds are in output 1, deposit tx input creation depends on the reserve + // being output 1. The amount has to be larger than the BSQ input to make sure the inputs get burnt. + // The BTC changeAddress is used, so it might get used for both output 0 and output 2. + if (!hasBsqOutputs) { + var bsqInputValue = preparedBsqTx.getInputs().stream() + .map(TransactionInput::getValue) + .reduce(Coin.valueOf(0), Coin::add); + + preparedBsqTx.addOutput(bsqInputValue.add(Coin.valueOf(1)), changeAddress); + } + // the reserved amount we need for the trade we send to our trade reservedForTradeAddress + preparedBsqTx.addOutput(reservedFundsForOffer, reservedForTradeAddress); - WalletService.checkWalletConsistency(wallet); - WalletService.verifyTransaction(resultTx); + // we allow spending of unconfirmed tx (double spend risk is low and usability would suffer if we need to + // wait for 1 confirmation) + // In case of double spend we will detect later in the trade process and use a ban score to penalize bad behaviour (not impl. yet) - WalletService.printTx(Res.getBaseCurrencyCode() + " wallet: Signed tx", resultTx); - return resultTx; + SendRequest sendRequest = SendRequest.forTx(preparedBsqTx); + sendRequest.shuffleOutputs = false; + sendRequest.aesKey = aesKey; + if (useSavingsWallet) { + sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE), + preferences.getIgnoreDustThreshold()); + } else { + sendRequest.coinSelector = new BtcCoinSelector(fundingAddress, preferences.getIgnoreDustThreshold()); + } + // We use a fixed fee + sendRequest.fee = txFee; + sendRequest.feePerKb = Coin.ZERO; + sendRequest.ensureMinRequiredFee = false; + + sendRequest.signInputs = false; + + // Change is optional in case of overpay or use of funds from savings wallet + sendRequest.changeAddress = changeAddress; + + checkNotNull(wallet, "Wallet must not be null"); + wallet.completeTx(sendRequest); + Transaction resultTx = sendRequest.tx; + removeDust(resultTx); + + // Sign all BTC inputs + for (int i = preparedBsqTxInputsSize; i < resultTx.getInputs().size(); i++) { + TransactionInput txIn = resultTx.getInputs().get(i); + checkArgument(txIn.getConnectedOutput() != null && + txIn.getConnectedOutput().isMine(wallet), + "txIn.getConnectedOutput() is not in our wallet. That must not happen."); + WalletService.signTransactionInput(wallet, aesKey, resultTx, txIn, i); + WalletService.checkScriptSig(resultTx, txIn, i); + } + + WalletService.checkWalletConsistency(wallet); + WalletService.verifyTransaction(resultTx); + + WalletService.printTx(Res.getBaseCurrencyCode() + " wallet: Signed tx", resultTx); + return resultTx; + } catch (Throwable t) { + log.error("completeBsqTradingFeeTx: preparedBsqTx={}", preparedBsqTx.toString()); + throw t; + } } From 25d227556b785c9e82d7ccdaac94bb428d8dccd2 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 7 Dec 2020 20:25:29 -0500 Subject: [PATCH 19/21] Add "Raw deposit transaction as hex" to details window Rename Contract as json button to detail data --- .../resources/i18n/displayStrings.properties | 1 + .../overlays/windows/TradeDetailsWindow.java | 20 ++++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 9c621af54a9..4022bfd536d 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -2643,6 +2643,7 @@ tradeDetailsWindow.tradingPeersOnion=Trading peers onion address tradeDetailsWindow.tradingPeersPubKeyHash=Trading peers pubkey hash tradeDetailsWindow.tradeState=Trade state tradeDetailsWindow.agentAddresses=Arbitrator/Mediator +tradeDetailsWindow.detailData=Detail data walletPasswordWindow.headline=Enter password to unlock diff --git a/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java b/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java index 494dad9d02c..f68c3bfb4b4 100644 --- a/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java +++ b/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java @@ -320,7 +320,7 @@ private void addContent() { } Tuple3 tuple = add2ButtonsWithBox(gridPane, ++rowIndex, - Res.get("shared.viewContractAsJson"), Res.get("shared.close"), 15, false); + Res.get("tradeDetailsWindow.detailData"), Res.get("shared.close"), 15, false); Button viewContractButton = tuple.first; viewContractButton.setMaxWidth(Region.USE_COMPUTED_SIZE); Button closeButton = tuple.second; @@ -335,15 +335,21 @@ private void addContent() { viewContractButton.setOnAction(e -> { TextArea textArea = new BisqTextArea(); textArea.setText(trade.getContractAsJson()); - String contractAsJson = trade.getContractAsJson(); - contractAsJson += "\n\nBuyerMultiSigPubKeyHex: " + Utils.HEX.encode(contract.getBuyerMultiSigPubKey()); - contractAsJson += "\nSellerMultiSigPubKeyHex: " + Utils.HEX.encode(contract.getSellerMultiSigPubKey()); + String data = "Contract as json:\n"; + data += trade.getContractAsJson(); + data += "\n\nBuyerMultiSigPubKeyHex: " + Utils.HEX.encode(contract.getBuyerMultiSigPubKey()); + data += "\nSellerMultiSigPubKeyHex: " + Utils.HEX.encode(contract.getSellerMultiSigPubKey()); if (CurrencyUtil.isFiatCurrency(offer.getCurrencyCode())) { - contractAsJson += "\nBuyersAccountAge: " + buyersAccountAge; - contractAsJson += "\nSellersAccountAge: " + sellersAccountAge; + data += "\n\nBuyersAccountAge: " + buyersAccountAge; + data += "\nSellersAccountAge: " + sellersAccountAge; } - textArea.setText(contractAsJson); + if (depositTx != null) { + String depositTxAsHex = Utils.HEX.encode(depositTx.bitcoinSerialize(true)); + data += "\n\nRaw deposit transaction as hex:\n" + depositTxAsHex; + } + + textArea.setText(data); textArea.setPrefHeight(50); textArea.setEditable(false); textArea.setWrapText(true); From 455c9b0ff31f34c8388532387810f10793731701 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 8 Dec 2020 10:13:31 -0500 Subject: [PATCH 20/21] Avoid logging warning at startup --- core/src/main/java/bisq/core/btc/model/AddressEntry.java | 4 ++++ core/src/main/java/bisq/core/btc/model/AddressEntryList.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntry.java b/core/src/main/java/bisq/core/btc/model/AddressEntry.java index fe73686d9ca..18bb07fcc28 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -204,6 +204,10 @@ public Address getAddress() { return address; } + public boolean isAddressNull() { + return address == null; + } + public boolean isOpenOffer() { return context == Context.OFFER_FUNDING || context == Context.RESERVED_FOR_TRADE; } diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java index 3453ab43dce..be9dd78fb4b 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -118,7 +118,7 @@ public void onWalletReady(Wallet wallet) { Address addressFromKey = Address.fromKey(Config.baseCurrencyNetworkParameters(), keyFromPubHash, scriptType); // We want to ensure key and address matches in case we have address in entry available already - if (addressEntry.getAddress() == null || addressFromKey.equals(addressEntry.getAddress())) { + if (addressEntry.isAddressNull() || addressFromKey.equals(addressEntry.getAddress())) { addressEntry.setDeterministicKey(keyFromPubHash); } else { log.error("We found an address entry without key but cannot apply the key as the address " + From a04cc7bfe22599e94fd5b98fed132f354a29d510 Mon Sep 17 00:00:00 2001 From: Christoph Atteneder Date: Tue, 8 Dec 2020 22:14:56 +0100 Subject: [PATCH 21/21] Revert to SNAPSHOT version --- build.gradle | 2 +- desktop/package/linux/Dockerfile | 2 +- desktop/package/linux/package.sh | 2 +- desktop/package/linux/release.sh | 2 +- desktop/package/macosx/create_app.sh | 2 +- desktop/package/macosx/finalize.sh | 2 +- desktop/package/macosx/insert_snapshot_version.sh | 2 +- desktop/package/windows/package.bat | 2 +- desktop/package/windows/release.bat | 2 +- relay/src/main/resources/version.txt | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index 95149e9cd9e..8b1004db5be 100644 --- a/build.gradle +++ b/build.gradle @@ -386,7 +386,7 @@ configure(project(':desktop')) { apply plugin: 'witness' apply from: '../gradle/witness/gradle-witness.gradle' - version = '1.5.1' + version = '1.5.1-SNAPSHOT' mainClassName = 'bisq.desktop.app.BisqAppMain' diff --git a/desktop/package/linux/Dockerfile b/desktop/package/linux/Dockerfile index 8aee1e69c03..52b22011cbc 100644 --- a/desktop/package/linux/Dockerfile +++ b/desktop/package/linux/Dockerfile @@ -8,7 +8,7 @@ # pull base image FROM openjdk:8-jdk -ENV version 1.5.1 +ENV version 1.5.1-SNAPSHOT RUN apt-get update && apt-get install -y --no-install-recommends openjfx && rm -rf /var/lib/apt/lists/* && apt-get install -y vim fakeroot diff --git a/desktop/package/linux/package.sh b/desktop/package/linux/package.sh index c95027f33ae..5f6641aadef 100755 --- a/desktop/package/linux/package.sh +++ b/desktop/package/linux/package.sh @@ -6,7 +6,7 @@ # - Update version below # - Ensure JAVA_HOME below is pointing to OracleJDK 10 directory -version=1.5.1 +version=1.5.1-SNAPSHOT version_base=$(echo $version | awk -F'[_-]' '{print $1}') if [ ! -f "$JAVA_HOME/bin/javapackager" ]; then if [ -d "/usr/lib/jvm/jdk-10.0.2" ]; then diff --git a/desktop/package/linux/release.sh b/desktop/package/linux/release.sh index 49b5964d34b..2f00bab04c0 100755 --- a/desktop/package/linux/release.sh +++ b/desktop/package/linux/release.sh @@ -4,7 +4,7 @@ # Prior to running this script: # - Update version below -version=1.5.1 +version=1.5.1-SNAPSHOT base_dir=$( cd "$(dirname "$0")" ; pwd -P )/../../.. package_dir=$base_dir/desktop/package release_dir=$base_dir/desktop/release/$version diff --git a/desktop/package/macosx/create_app.sh b/desktop/package/macosx/create_app.sh index 6c405a84241..57c712a1060 100755 --- a/desktop/package/macosx/create_app.sh +++ b/desktop/package/macosx/create_app.sh @@ -6,7 +6,7 @@ mkdir -p deploy set -e -version="1.5.1" +version="1.5.1-SNAPSHOT" cd .. ./gradlew :desktop:build -x test shadowJar diff --git a/desktop/package/macosx/finalize.sh b/desktop/package/macosx/finalize.sh index 55b2181508c..fa3a8a75445 100755 --- a/desktop/package/macosx/finalize.sh +++ b/desktop/package/macosx/finalize.sh @@ -2,7 +2,7 @@ cd ../../ -version="1.5.1" +version="1.5.1-SNAPSHOT" target_dir="releases/$version" diff --git a/desktop/package/macosx/insert_snapshot_version.sh b/desktop/package/macosx/insert_snapshot_version.sh index 604e5575589..b034b42f777 100755 --- a/desktop/package/macosx/insert_snapshot_version.sh +++ b/desktop/package/macosx/insert_snapshot_version.sh @@ -2,7 +2,7 @@ cd $(dirname $0)/../../../ -version=1.5.0 +version=1.5.1 find . -type f \( -name "finalize.sh" \ -o -name "create_app.sh" \ diff --git a/desktop/package/windows/package.bat b/desktop/package/windows/package.bat index 3c86e20dd80..8d0af674d8b 100644 --- a/desktop/package/windows/package.bat +++ b/desktop/package/windows/package.bat @@ -11,7 +11,7 @@ @echo off -set version=1.5.1 +set version=1.5.1-SNAPSHOT if not exist "%JAVA_HOME%\bin\javapackager.exe" ( if not exist "%ProgramFiles%\Java\jdk-10.0.2" ( echo Javapackager not found. Update JAVA_HOME variable to point to OracleJDK. diff --git a/desktop/package/windows/release.bat b/desktop/package/windows/release.bat index 7b3caef996b..f79d4184fd8 100644 --- a/desktop/package/windows/release.bat +++ b/desktop/package/windows/release.bat @@ -6,7 +6,7 @@ @echo off -set version=1.5.1 +set version=1.5.1-SNAPSHOT set release_dir=%~dp0..\..\..\releases\%version% set package_dir=%~dp0.. diff --git a/relay/src/main/resources/version.txt b/relay/src/main/resources/version.txt index 26ca594609a..f825f7c7f1b 100644 --- a/relay/src/main/resources/version.txt +++ b/relay/src/main/resources/version.txt @@ -1 +1 @@ -1.5.1 +1.5.1-SNAPSHOT