From f13a7b164cebf632d198be65aa2f4ad746c993fc Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 14:53:17 -0300 Subject: [PATCH 01/30] Create a P2WPKH keychain for new btc wallets --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 acc33637023..dea3b7591a9 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -359,9 +359,7 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i } protected Wallet createWallet(boolean isBsqWallet) { - // Change preferredOutputScriptType of btc wallet to P2WPKH to start using segwit - // Script.ScriptType preferredOutputScriptType = isBsqWallet ? Script.ScriptType.P2PKH : Script.ScriptType.P2WPKH; - Script.ScriptType preferredOutputScriptType = Script.ScriptType.P2PKH; + Script.ScriptType preferredOutputScriptType = isBsqWallet ? Script.ScriptType.P2PKH : Script.ScriptType.P2WPKH; KeyChainGroupStructure structure = new BisqKeyChainGroupStructure(isBsqWallet); KeyChainGroup.Builder kcgBuilder = KeyChainGroup.builder(params, structure); if (restoreFromSeed != null) { From cd286600697f6cffc76ea9b84b0f6b6d3fe4e041 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 14:53:37 -0300 Subject: [PATCH 02/30] Add a P2WPKH keychain for existing wallets --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 dea3b7591a9..d190f37170c 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -352,6 +352,14 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i wallet = serializer.readWallet(params, extArray, proto); if (shouldReplayWallet) wallet.reset(); + if (!isBsqWallet && BisqKeyChainGroupStructure.BIP44_BTC_NON_SEGWIT_ACCOUNT_PATH.equals(wallet.getActiveKeyChain().getAccountPath())) { + // Btc wallet does not have a native segwit keychain, we should add one. + DeterministicSeed seed = wallet.getKeyChainSeed(); + DeterministicKeyChain nativeSegwitKeyChain = DeterministicKeyChain.builder().seed(seed) + .outputScriptType(Script.ScriptType.P2WPKH) + .accountPath(new BisqKeyChainGroupStructure(isBsqWallet).accountPathFor(Script.ScriptType.P2WPKH)).build(); + wallet.addAndActivateHDChain(nativeSegwitKeyChain); + } } finally { walletStream.close(); } From ced357ce258a1538b592b5f00e703eab12619e1e Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 17:42:31 -0300 Subject: [PATCH 03/30] AddressEntry: Add boolean segwit flag --- .../bisq/core/btc/model/AddressEntry.java | 25 +++++++++++++------ .../bisq/core/btc/model/AddressEntryList.java | 20 +++++++++------ .../core/btc/wallet/BtcWalletService.java | 22 ++++++++-------- proto/src/main/proto/pb.proto | 1 + 4 files changed, 42 insertions(+), 26 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 3b036b47a43..4e40c5d0a5d 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -28,6 +28,7 @@ import org.bitcoinj.core.Coin; import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.crypto.DeterministicKey; +import org.bitcoinj.script.Script; import java.util.Optional; @@ -74,6 +75,9 @@ public enum Context { private long coinLockedInMultiSig; + @Getter + private boolean segwit; + @Nullable transient private DeterministicKey keyPair; @Nullable @@ -86,18 +90,20 @@ public enum Context { // Constructor, initialization /////////////////////////////////////////////////////////////////////////////////////////// - public AddressEntry(DeterministicKey keyPair, Context context) { - this(keyPair, context, null); + public AddressEntry(DeterministicKey keyPair, Context context, boolean segwit) { + this(keyPair, context, null, segwit); } public AddressEntry(@NotNull DeterministicKey keyPair, Context context, - @Nullable String offerId) { + @Nullable String offerId, + boolean segwit) { this.keyPair = keyPair; this.context = context; this.offerId = offerId; pubKey = keyPair.getPubKey(); pubKeyHash = keyPair.getPubKeyHash(); + this.segwit = segwit; } @@ -109,12 +115,14 @@ private AddressEntry(byte[] pubKey, byte[] pubKeyHash, Context context, @Nullable String offerId, - Coin coinLockedInMultiSig) { + Coin coinLockedInMultiSig, + boolean segwit) { this.pubKey = pubKey; this.pubKeyHash = pubKeyHash; this.context = context; this.offerId = offerId; this.coinLockedInMultiSig = coinLockedInMultiSig.value; + this.segwit = segwit; } public static AddressEntry fromProto(protobuf.AddressEntry proto) { @@ -122,7 +130,8 @@ 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())); + Coin.valueOf(proto.getCoinLockedInMultiSig()), + proto.getSegwit()); } @Override @@ -131,7 +140,8 @@ public protobuf.AddressEntry toProtoMessage() { .setPubKey(ByteString.copyFrom(pubKey)) .setPubKeyHash(ByteString.copyFrom(pubKeyHash)) .setContext(protobuf.AddressEntry.Context.valueOf(context.name())) - .setCoinLockedInMultiSig(coinLockedInMultiSig); + .setCoinLockedInMultiSig(coinLockedInMultiSig) + .setSegwit(segwit); Optional.ofNullable(offerId).ifPresent(builder::setOfferId); return builder.build(); } @@ -175,7 +185,7 @@ public String getAddressString() { @Nullable public Address getAddress() { if (address == null && keyPair != null) - address = LegacyAddress.fromKey(Config.baseCurrencyNetworkParameters(), keyPair); + address = Address.fromKey(Config.baseCurrencyNetworkParameters(), keyPair, segwit ? Script.ScriptType.P2WPKH : Script.ScriptType.P2PKH); return address; } @@ -198,6 +208,7 @@ public String toString() { ", context=" + context + ", offerId='" + offerId + '\'' + ", coinLockedInMultiSig=" + coinLockedInMultiSig + + ", segwit=" + segwit + "}"; } } 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 4827fbc1621..cdf83a2c30d 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -26,6 +26,7 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.LegacyAddress; +import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Transaction; import org.bitcoinj.crypto.DeterministicKey; import org.bitcoinj.script.Script; @@ -35,6 +36,8 @@ import com.google.common.collect.ImmutableList; +import org.apache.commons.lang3.tuple.Pair; + import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -133,7 +136,7 @@ public void onWalletReady(Wallet wallet) { toBeRemoved.forEach(entrySet::remove); } else { // As long the old arbitration domain is not removed from the code base we still support it here. - entrySet.add(new AddressEntry(wallet.freshReceiveKey(), AddressEntry.Context.ARBITRATOR)); + entrySet.add(new AddressEntry(wallet.freshReceiveKey(), AddressEntry.Context.ARBITRATOR, true)); } // In case we restore from seed words and have balance we need to add the relevant addresses to our list. @@ -147,7 +150,7 @@ public void onWalletReady(Wallet wallet) { DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(address); if (key != null) { // Address will be derived from key in getAddress method - entrySet.add(new AddressEntry(key, AddressEntry.Context.AVAILABLE)); + entrySet.add(new AddressEntry(key, AddressEntry.Context.AVAILABLE, address instanceof SegwitAddress)); } }); } @@ -192,7 +195,8 @@ public void addAddressEntry(AddressEntry addressEntry) { public void swapToAvailable(AddressEntry addressEntry) { boolean setChangedByRemove = entrySet.remove(addressEntry); boolean setChangedByAdd = entrySet.add(new AddressEntry(addressEntry.getKeyPair(), - AddressEntry.Context.AVAILABLE)); + AddressEntry.Context.AVAILABLE, + addressEntry.isSegwit())); if (setChangedByRemove || setChangedByAdd) { requestPersistence(); } @@ -202,7 +206,7 @@ public AddressEntry swapAvailableToAddressEntryWithOfferId(AddressEntry addressE AddressEntry.Context context, String offerId) { boolean setChangedByRemove = entrySet.remove(addressEntry); - final AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId); + final AddressEntry newAddressEntry = new AddressEntry(addressEntry.getKeyPair(), context, offerId, addressEntry.isSegwit()); boolean setChangedByAdd = entrySet.add(newAddressEntry); if (setChangedByRemove || setChangedByAdd) requestPersistence(); @@ -225,10 +229,10 @@ private void maybeAddNewAddressEntry(Transaction tx) { .map(output -> output.getScriptPubKey().getToAddress(wallet.getNetworkParameters())) .filter(Objects::nonNull) .filter(this::isAddressNotInEntries) - .map(address -> (DeterministicKey) wallet.findKeyFromPubKeyHash(address.getHash(), - Script.ScriptType.P2PKH)) - .filter(Objects::nonNull) - .map(deterministicKey -> new AddressEntry(deterministicKey, AddressEntry.Context.AVAILABLE)) + .map(address -> Pair.of(address, (DeterministicKey) wallet.findKeyFromAddress(address))) + .filter(pair -> pair.getRight() != null) + .map(pair -> new AddressEntry(pair.getRight(), AddressEntry.Context.AVAILABLE, + pair.getLeft() instanceof SegwitAddress)) .forEach(this::addAddressEntry); } 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 b5860e00491..2f0a508803a 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -584,23 +584,13 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context if (emptyAvailableAddressEntry.isPresent()) { return addressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); } else { - AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context, offerId); + AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context, offerId, true); addressEntryList.addAddressEntry(entry); return entry; } } } - private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, Optional addressEntry) { - if (addressEntry.isPresent()) { - return addressEntry.get(); - } else { - AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context); - addressEntryList.addAddressEntry(entry); - return entry; - } - } - public AddressEntry getArbitratorAddressEntry() { AddressEntry.Context context = AddressEntry.Context.ARBITRATOR; Optional addressEntry = getAddressEntryListAsImmutableList().stream() @@ -623,6 +613,16 @@ public void recoverAddressEntry(String offerId, String address, AddressEntry.Con addressEntryList.swapAvailableToAddressEntryWithOfferId(addressEntry, context, offerId)); } + private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, Optional addressEntry) { + if (addressEntry.isPresent()) { + return addressEntry.get(); + } else { + AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context, true); + addressEntryList.addAddressEntry(entry); + return entry; + } + } + private Optional findAddressEntry(String address, AddressEntry.Context context) { return getAddressEntryListAsImmutableList().stream() .filter(e -> address.equals(e.getAddressString())) diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index e16746c09be..11c32109811 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -1239,6 +1239,7 @@ message AddressEntry { bytes pub_key = 9; bytes pub_key_hash = 10; int64 coin_locked_in_multi_sig = 11; + bool segwit = 12; } message NavigationPath { From e85c66b8103940975ae3562110c5ea50137fc368 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 18:33:08 -0300 Subject: [PATCH 04/30] Stop using LegacyAddress for btc addresses --- .../bisq/core/btc/model/AddressEntryList.java | 9 +++---- .../core/btc/wallet/BtcWalletService.java | 4 +++- .../core/btc/wallet/TradeWalletService.java | 24 +++++++++---------- 3 files changed, 20 insertions(+), 17 deletions(-) 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 cdf83a2c30d..9b1e09cf765 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -25,7 +25,6 @@ import com.google.protobuf.Message; import org.bitcoinj.core.Address; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Transaction; import org.bitcoinj.crypto.DeterministicKey; @@ -110,11 +109,13 @@ public void onWalletReady(Wallet wallet) { if (!entrySet.isEmpty()) { Set toBeRemoved = new HashSet<>(); entrySet.forEach(addressEntry -> { + Script.ScriptType scriptType = addressEntry.isSegwit() ? Script.ScriptType.P2WPKH + : Script.ScriptType.P2PKH; DeterministicKey keyFromPubHash = (DeterministicKey) wallet.findKeyFromPubKeyHash( - addressEntry.getPubKeyHash(), - Script.ScriptType.P2PKH); + addressEntry.getPubKeyHash(), scriptType); if (keyFromPubHash != null) { - Address addressFromKey = LegacyAddress.fromKey(Config.baseCurrencyNetworkParameters(), keyFromPubHash); + 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())) { addressEntry.setDeterministicKey(keyFromPubHash); 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 2f0a508803a..c0a336580b7 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -32,8 +32,10 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.AddressFormatException; import org.bitcoinj.core.Coin; +import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; import org.bitcoinj.core.LegacyAddress; +import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.core.TransactionInput; @@ -998,7 +1000,7 @@ private boolean feeEstimationNotSatisfied(int counter, Transaction tx) { public int getEstimatedFeeTxSize(List outputValues, Coin txFee) throws InsufficientMoneyException, AddressFormatException { Transaction transaction = new Transaction(params); - Address dummyAddress = LegacyAddress.fromKey(params, wallet.currentReceiveKey()); + Address dummyAddress = SegwitAddress.fromKey(params, new ECKey()); outputValues.forEach(outputValue -> transaction.addOutput(outputValue, dummyAddress)); SendRequest sendRequest = SendRequest.forTx(transaction); 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 f1ea7b042cb..48b4ff50ca5 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -36,8 +36,8 @@ import org.bitcoinj.core.Coin; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.NetworkParameters; +import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Sha256Hash; import org.bitcoinj.core.SignatureDecodeException; import org.bitcoinj.core.Transaction; @@ -153,7 +153,7 @@ public Transaction createBtcTradingFeeTx(Address fundingAddress, Transaction tradingFeeTx = new Transaction(params); SendRequest sendRequest = null; try { - tradingFeeTx.addOutput(tradingFee, LegacyAddress.fromBase58(params, feeReceiverAddress)); + tradingFeeTx.addOutput(tradingFee, Address.fromString(params, feeReceiverAddress)); // the reserved amount we need for the trade we send to our trade reservedForTradeAddress tradingFeeTx.addOutput(reservedFundsForOffer, reservedForTradeAddress); @@ -335,7 +335,7 @@ OUT[0] dummyOutputAmount (inputAmount - tx fee) Transaction dummyTX = new Transaction(params); // The output is just used to get the right inputs and change outputs, so we use an anonymous ECKey, as it will never be used for anything. // We don't care about fee calculation differences between the real tx and that dummy tx as we use a static tx fee. - TransactionOutput dummyOutput = new TransactionOutput(params, dummyTX, dummyOutputAmount, LegacyAddress.fromKey(params, new ECKey())); + TransactionOutput dummyOutput = new TransactionOutput(params, dummyTX, dummyOutputAmount, SegwitAddress.fromKey(params, new ECKey())); dummyTX.addOutput(dummyOutput); // Find the needed inputs to pay the output, optionally add 1 change output. @@ -454,7 +454,7 @@ private PreparedDepositTxAndMakerInputs makerCreatesDepositTx(boolean makerIsBuy // First we construct a dummy TX to get the inputs and outputs we want to use for the real deposit tx. // Similar to the way we did in the createTakerDepositTxInputs method. Transaction dummyTx = new Transaction(params); - TransactionOutput dummyOutput = new TransactionOutput(params, dummyTx, makerInputAmount, LegacyAddress.fromKey(params, new ECKey())); + TransactionOutput dummyOutput = new TransactionOutput(params, dummyTx, makerInputAmount, SegwitAddress.fromKey(params, new ECKey())); dummyTx.addOutput(dummyOutput); addAvailableInputsAndChangeOutputs(dummyTx, makerAddress, makerChangeAddress); // Normally we have only 1 input but we support multiple inputs if the user has paid in with several transactions. @@ -516,7 +516,7 @@ private PreparedDepositTxAndMakerInputs makerCreatesDepositTx(boolean makerIsBuy TransactionOutput takerTransactionOutput = null; if (takerChangeOutputValue > 0 && takerChangeAddressString != null) { takerTransactionOutput = new TransactionOutput(params, preparedDepositTx, Coin.valueOf(takerChangeOutputValue), - LegacyAddress.fromBase58(params, takerChangeAddressString)); + Address.fromString(params, takerChangeAddressString)); } if (makerIsBuyer) { @@ -686,7 +686,7 @@ public Transaction createDelayedUnsignedPayoutTx(Transaction depositTx, delayedPayoutTx.addInput(p2SHMultiSigOutput); applyLockTime(lockTime, delayedPayoutTx); Coin outputAmount = p2SHMultiSigOutput.getValue().subtract(minerFee); - delayedPayoutTx.addOutput(outputAmount, LegacyAddress.fromBase58(params, donationAddressString)); + delayedPayoutTx.addOutput(outputAmount, Address.fromString(params, donationAddressString)); WalletService.printTx("Unsigned delayedPayoutTx ToDonationAddress", delayedPayoutTx); WalletService.verifyTransaction(delayedPayoutTx); return delayedPayoutTx; @@ -938,10 +938,10 @@ public Transaction traderSignAndFinalizeDisputedPayoutTx(byte[] depositTxSeriali Transaction payoutTx = new Transaction(params); payoutTx.addInput(p2SHMultiSigOutput); if (buyerPayoutAmount.isPositive()) { - payoutTx.addOutput(buyerPayoutAmount, LegacyAddress.fromBase58(params, buyerAddressString)); + payoutTx.addOutput(buyerPayoutAmount, Address.fromString(params, buyerAddressString)); } if (sellerPayoutAmount.isPositive()) { - payoutTx.addOutput(sellerPayoutAmount, LegacyAddress.fromBase58(params, sellerAddressString)); + payoutTx.addOutput(sellerPayoutAmount, Address.fromString(params, sellerAddressString)); } // take care of sorting! @@ -1001,10 +1001,10 @@ public void emergencySignAndPublishPayoutTxFrom2of2MultiSig(String depositTxHex, payoutTx.addInput(new TransactionInput(params, depositTx, p2SHMultiSigOutputScript.getProgram(), new TransactionOutPoint(params, 0, spendTxHash), msOutput)); if (buyerPayoutAmount.isPositive()) { - payoutTx.addOutput(buyerPayoutAmount, LegacyAddress.fromBase58(params, buyerAddressString)); + payoutTx.addOutput(buyerPayoutAmount, Address.fromString(params, buyerAddressString)); } if (sellerPayoutAmount.isPositive()) { - payoutTx.addOutput(sellerPayoutAmount, LegacyAddress.fromBase58(params, sellerAddressString)); + payoutTx.addOutput(sellerPayoutAmount, Address.fromString(params, sellerAddressString)); } // take care of sorting! @@ -1146,10 +1146,10 @@ private Transaction createPayoutTx(Transaction depositTx, Transaction transaction = new Transaction(params); transaction.addInput(p2SHMultiSigOutput); if (buyerPayoutAmount.isPositive()) { - transaction.addOutput(buyerPayoutAmount, LegacyAddress.fromBase58(params, buyerAddressString)); + transaction.addOutput(buyerPayoutAmount, Address.fromString(params, buyerAddressString)); } if (sellerPayoutAmount.isPositive()) { - transaction.addOutput(sellerPayoutAmount, LegacyAddress.fromBase58(params, sellerAddressString)); + transaction.addOutput(sellerPayoutAmount, Address.fromString(params, sellerAddressString)); } checkArgument(transaction.getOutputs().size() >= 1, "We need at least one output."); return transaction; From 0c7f3456b6561811cc1c5f3a90b39f5ce9a3733b Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 18:44:42 -0300 Subject: [PATCH 05/30] Fix log msg in BtcCoinSelector --- core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java b/core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java index f0df0adb35d..91b0c84c57b 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java @@ -64,7 +64,7 @@ protected boolean isTxOutputSpendable(TransactionOutput output) { Address address = WalletService.getAddressFromOutput(output); return addresses.contains(address); } else { - log.warn("transactionOutput.getScriptPubKey() not isSentToAddress or isPayToScriptHash"); + log.warn("transactionOutput.getScriptPubKey() is not P2PKH nor P2SH nor P2WH"); return false; } } From 0f4c66f43d723f597e9646479d3776419681dc7e Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 21:04:58 -0300 Subject: [PATCH 06/30] Comment out segwit BSQ account path Segwit is not used for BSQ --- .../core/btc/setup/BisqKeyChainGroupStructure.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java b/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java index 7c7f6a4e3a8..66ec8b7cea2 100644 --- a/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java +++ b/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java @@ -47,10 +47,11 @@ public class BisqKeyChainGroupStructure implements KeyChainGroupStructure { new ChildNumber(142, true), ChildNumber.ZERO_HARDENED); - public static final ImmutableList BIP44_BSQ_SEGWIT_ACCOUNT_PATH = ImmutableList.of( - new ChildNumber(44, true), - new ChildNumber(142, true), - ChildNumber.ONE_HARDENED); + // We don't use segwit for BSQ + // public static final ImmutableList BIP44_BSQ_SEGWIT_ACCOUNT_PATH = ImmutableList.of( + // new ChildNumber(44, true), + // new ChildNumber(142, true), + // ChildNumber.ONE_HARDENED); private boolean isBsqWallet; @@ -71,7 +72,8 @@ else if (outputScriptType == Script.ScriptType.P2WPKH) if (outputScriptType == null || outputScriptType == Script.ScriptType.P2PKH) return BIP44_BSQ_NON_SEGWIT_ACCOUNT_PATH; else if (outputScriptType == Script.ScriptType.P2WPKH) - return BIP44_BSQ_SEGWIT_ACCOUNT_PATH; + //return BIP44_BSQ_SEGWIT_ACCOUNT_PATH; + throw new IllegalArgumentException(outputScriptType.toString()); else throw new IllegalArgumentException(outputScriptType.toString()); } From 25515710ab724203690de8d7057d7bbd6903a094 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 21:05:24 -0300 Subject: [PATCH 07/30] TradeWalletService: adapt to segwit wallet --- .../core/btc/wallet/TradeWalletService.java | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 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 48b4ff50ca5..d0fbd194607 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -36,6 +36,7 @@ import org.bitcoinj.core.Coin; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; +import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Sha256Hash; @@ -44,6 +45,7 @@ import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; +import org.bitcoinj.core.TransactionWitness; import org.bitcoinj.core.Utils; import org.bitcoinj.crypto.DeterministicKey; import org.bitcoinj.crypto.TransactionSignature; @@ -601,6 +603,9 @@ public Transaction takerSignsDepositTx(boolean takerIsSeller, for (int i = 0; i < buyerInputs.size(); i++) { TransactionInput transactionInput = makersDepositTx.getInputs().get(i); depositTx.addInput(getTransactionInput(depositTx, getMakersScriptSigProgram(transactionInput), buyerInputs.get(i))); + if (!TransactionWitness.EMPTY.equals(transactionInput.getWitness())) { + depositTx.getInput(depositTx.getInputs().size()-1).setWitness(transactionInput.getWitness()); + } } // Add seller inputs @@ -665,6 +670,10 @@ public void sellerAsMakerFinalizesDepositTx(Transaction myDepositTx, TransactionInput input = takersDepositTx.getInput(i); Script scriptSig = input.getScriptSig(); myDepositTx.getInput(i).setScriptSig(scriptSig); + TransactionWitness witness = input.getWitness(); + if (!TransactionWitness.EMPTY.equals(witness)) { + myDepositTx.getInput(i).setWitness(witness); + } } WalletService.printTx("sellerAsMakerFinalizesDepositTx", myDepositTx); @@ -1165,13 +1174,27 @@ private void signInput(Transaction transaction, TransactionInput input, int inpu if (sigKey.isEncrypted()) { checkNotNull(aesKey); } - Sha256Hash hash = transaction.hashForSignature(inputIndex, scriptPubKey, Transaction.SigHash.ALL, false); - ECKey.ECDSASignature signature = sigKey.sign(hash, aesKey); - TransactionSignature txSig = new TransactionSignature(signature, Transaction.SigHash.ALL, false); - if (ScriptPattern.isP2PK(scriptPubKey)) { - input.setScriptSig(ScriptBuilder.createInputScript(txSig)); - } else if (ScriptPattern.isP2PKH(scriptPubKey)) { - input.setScriptSig(ScriptBuilder.createInputScript(txSig, sigKey)); + + if (ScriptPattern.isP2PK(scriptPubKey) || ScriptPattern.isP2PKH(scriptPubKey)) { + Sha256Hash hash = transaction.hashForSignature(inputIndex, scriptPubKey, Transaction.SigHash.ALL, false); + ECKey.ECDSASignature signature = sigKey.sign(hash, aesKey); + TransactionSignature txSig = new TransactionSignature(signature, Transaction.SigHash.ALL, false); + if (ScriptPattern.isP2PK(scriptPubKey)) { + input.setScriptSig(ScriptBuilder.createInputScript(txSig)); + } else if (ScriptPattern.isP2PKH(scriptPubKey)) { + input.setScriptSig(ScriptBuilder.createInputScript(txSig, sigKey)); + } + } else if (ScriptPattern.isP2WPKH(scriptPubKey)) { + // TODO: Consider using this alternative way to build the scriptCode (taken from bitcoinj master) + // Script scriptCode = ScriptBuilder.createP2PKHOutputScript(sigKey) + Script scriptCode = new ScriptBuilder().data( + ScriptBuilder.createOutputScript(LegacyAddress.fromKey(transaction.getParams(), sigKey)).getProgram()) + .build(); + Coin value = input.getValue(); + TransactionSignature txSig = transaction.calculateWitnessSignature(inputIndex, sigKey, scriptCode, value, + Transaction.SigHash.ALL, false); + input.setScriptSig(ScriptBuilder.createEmpty()); + input.setWitness(TransactionWitness.redeemP2WPKH(txSig, sigKey)); } else { throw new SigningException("Don't know how to sign for this kind of scriptPubKey: " + scriptPubKey); } From d8b755794e2d7e0fb2e541a5635d35c138c8ff6f Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 21:05:56 -0300 Subject: [PATCH 08/30] WalletService: adapt to segwit wallet --- .../bisq/core/btc/wallet/WalletService.java | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletService.java b/core/src/main/java/bisq/core/btc/wallet/WalletService.java index b92cdc24103..f17122ac3b3 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletService.java @@ -17,6 +17,7 @@ package bisq.core.btc.wallet; +import bisq.core.btc.exceptions.SigningException; import bisq.core.btc.exceptions.TransactionVerificationException; import bisq.core.btc.exceptions.WalletException; import bisq.core.btc.listeners.AddressConfidenceListener; @@ -37,12 +38,14 @@ import org.bitcoinj.core.Context; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; +import org.bitcoinj.core.LegacyAddress; 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.TransactionOutput; +import org.bitcoinj.core.TransactionWitness; import org.bitcoinj.core.VerificationException; import org.bitcoinj.core.listeners.NewBestBlockListener; import org.bitcoinj.core.listeners.TransactionConfidenceEventListener; @@ -51,6 +54,7 @@ import org.bitcoinj.crypto.KeyCrypterScrypt; import org.bitcoinj.crypto.TransactionSignature; import org.bitcoinj.script.Script; +import org.bitcoinj.script.ScriptBuilder; import org.bitcoinj.script.ScriptChunk; import org.bitcoinj.script.ScriptException; import org.bitcoinj.script.ScriptPattern; @@ -241,7 +245,7 @@ public static void checkScriptSig(Transaction transaction, int inputIndex) throws TransactionVerificationException { try { checkNotNull(input.getConnectedOutput(), "input.getConnectedOutput() must not be null"); - input.getScriptSig().correctlySpends(transaction, inputIndex, input.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); + input.getScriptSig().correctlySpends(transaction, inputIndex, input.getWitness(), input.getValue(), input.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); } catch (Throwable t) { t.printStackTrace(); log.error(t.getMessage()); @@ -265,7 +269,7 @@ public static void signTransactionInput(Wallet wallet, // We assume if it's already signed, it's hopefully got a SIGHASH type that will not invalidate when // we sign missing pieces (to check this would require either assuming any signatures are signing // standard output types or a way to get processed signatures out of script execution) - txIn.getScriptSig().correctlySpends(tx, index, txIn.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); + txIn.getScriptSig().correctlySpends(tx, index, txIn.getWitness(), txIn.getValue(), txIn.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); log.warn("Input {} already correctly spends output, assuming SIGHASH type used will be safe and skipping signing.", index); return; } catch (ScriptException e) { @@ -288,7 +292,7 @@ public static void signTransactionInput(Wallet wallet, // We assume if it's already signed, it's hopefully got a SIGHASH type that will not invalidate when // we sign missing pieces (to check this would require either assuming any signatures are signing // standard output types or a way to get processed signatures out of script execution) - txIn.getScriptSig().correctlySpends(tx, index, txIn.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); + txIn.getScriptSig().correctlySpends(tx, index, txIn.getWitness(), txIn.getValue(), txIn.getConnectedOutput().getScriptPubKey(), Script.ALL_VERIFY_FLAGS); log.warn("Input {} already correctly spends output, assuming SIGHASH type used will be safe and skipping signing.", index); return; } catch (ScriptException e) { @@ -312,14 +316,31 @@ public static void signTransactionInput(Wallet wallet, Script inputScript = txIn.getScriptSig(); byte[] script = redeemData.redeemScript.getProgram(); - try { - TransactionSignature signature = partialTx.calculateSignature(index, key, script, Transaction.SigHash.ALL, false); - inputScript = scriptPubKey.getScriptSigWithSignature(inputScript, signature.encodeToBitcoin(), 0); - txIn.setScriptSig(inputScript); - } catch (ECKey.KeyIsEncryptedException e1) { - throw e1; - } catch (ECKey.MissingPrivateKeyException e1) { - log.warn("No private key in keypair for input {}", index); + + if (ScriptPattern.isP2PK(scriptPubKey) || ScriptPattern.isP2PKH(scriptPubKey)) { + try { + TransactionSignature signature = partialTx.calculateSignature(index, key, script, Transaction.SigHash.ALL, false); + inputScript = scriptPubKey.getScriptSigWithSignature(inputScript, signature.encodeToBitcoin(), 0); + txIn.setScriptSig(inputScript); + } catch (ECKey.KeyIsEncryptedException e1) { + throw e1; + } catch (ECKey.MissingPrivateKeyException e1) { + log.warn("No private key in keypair for input {}", index); + } + } else if (ScriptPattern.isP2WPKH(scriptPubKey)) { + // TODO: Consider using this alternative way to build the scriptCode (taken from bitcoinj master) + // Script scriptCode = ScriptBuilder.createP2PKHOutputScript(key); + Script scriptCode = new ScriptBuilder().data( + ScriptBuilder.createOutputScript(LegacyAddress.fromKey(tx.getParams(), key)).getProgram()) + .build(); + Coin value = txIn.getValue(); + TransactionSignature txSig = tx.calculateWitnessSignature(index, key, scriptCode, value, + Transaction.SigHash.ALL, false); + txIn.setScriptSig(ScriptBuilder.createEmpty()); + txIn.setWitness(TransactionWitness.redeemP2WPKH(txSig, key)); + } else { + // log.error("Unexpected script type."); + throw new RuntimeException("Unexpected script type."); } } else { log.warn("Missing connected output, assuming input {} is already signed.", index); @@ -592,7 +613,7 @@ public DeterministicKey findKeyFromPubKeyHash(byte[] pubKeyHash) { @Nullable public DeterministicKey findKeyFromPubKey(byte[] pubKey) { - return wallet.getActiveKeyChain().findKeyFromPubKey(pubKey); + return (DeterministicKey) wallet.findKeyFromPubKey(pubKey); } public boolean isEncrypted() { From a3708485f950912d18a9d62c992d35c9e175e90c Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Fri, 25 Sep 2020 02:12:18 -0300 Subject: [PATCH 09/30] New AddressEntry: use different script types Use P2WPKH for AVAILABLE context and P2PKH for the rest. Disable reusing unused AVAILABLE entries until segwit support is mandatory in Bisq. --- .../core/btc/wallet/BtcWalletService.java | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 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 c0a336580b7..48a477125e0 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -43,6 +43,7 @@ import org.bitcoinj.core.TransactionOutput; import org.bitcoinj.crypto.DeterministicKey; import org.bitcoinj.crypto.KeyCrypterScrypt; +import org.bitcoinj.script.Script; import org.bitcoinj.script.ScriptBuilder; import org.bitcoinj.wallet.SendRequest; import org.bitcoinj.wallet.Wallet; @@ -578,18 +579,20 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context if (addressEntry.isPresent()) { return addressEntry.get(); } else { +// Disable reusing unused AVAILABLE entries until segwit support in mandatory in Bisq // We try to use available and not yet used entries - Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream() - .filter(e -> AddressEntry.Context.AVAILABLE == e.getContext()) - .filter(e -> isAddressUnused(e.getAddress())) - .findAny(); - if (emptyAvailableAddressEntry.isPresent()) { - return addressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); - } else { - AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context, offerId, true); +// Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream() +// .filter(e -> AddressEntry.Context.AVAILABLE == e.getContext()) +// .filter(e -> isAddressUnused(e.getAddress())) +// .findAny(); +// if (emptyAvailableAddressEntry.isPresent()) { +// return addressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); +// } else { + DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); + AddressEntry entry = new AddressEntry(key, context, offerId, false); addressEntryList.addAddressEntry(entry); return entry; - } +// } } } @@ -619,7 +622,16 @@ private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, Optio if (addressEntry.isPresent()) { return addressEntry.get(); } else { - AddressEntry entry = new AddressEntry(wallet.freshReceiveKey(), context, true); + DeterministicKey key; + boolean segwit; + if (AddressEntry.Context.AVAILABLE.equals(context)) { + key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2WPKH)); + segwit = true; + } else { + key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); + segwit = false; + } + AddressEntry entry = new AddressEntry(key, context, segwit); addressEntryList.addAddressEntry(entry); return entry; } From a9cc28f65a701288097fe166d6b68ace0a75574c Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Fri, 25 Sep 2020 02:12:52 -0300 Subject: [PATCH 10/30] AddressEntryList: arbitrator entry use P2PKH --- core/src/main/java/bisq/core/btc/model/AddressEntryList.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 9b1e09cf765..c88ebc40fbd 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -137,7 +137,8 @@ public void onWalletReady(Wallet wallet) { toBeRemoved.forEach(entrySet::remove); } else { // As long the old arbitration domain is not removed from the code base we still support it here. - entrySet.add(new AddressEntry(wallet.freshReceiveKey(), AddressEntry.Context.ARBITRATOR, true)); + DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); + entrySet.add(new AddressEntry(key, AddressEntry.Context.ARBITRATOR, false)); } // In case we restore from seed words and have balance we need to add the relevant addresses to our list. From f9f5d92941c49ecc24949c4b20b6b4c70de69729 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Fri, 25 Sep 2020 04:57:13 -0300 Subject: [PATCH 11/30] Add segwit/legacy checbox for address creation --- .../core/btc/wallet/BtcWalletService.java | 16 ++++++------ .../resources/i18n/displayStrings.properties | 1 + .../main/funds/deposit/DepositView.java | 25 ++++++++++++++----- 3 files changed, 29 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 48a477125e0..6603342fbff 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -601,16 +601,21 @@ public AddressEntry getArbitratorAddressEntry() { Optional addressEntry = getAddressEntryListAsImmutableList().stream() .filter(e -> context == e.getContext()) .findAny(); - return getOrCreateAddressEntry(context, addressEntry); + return getOrCreateAddressEntry(context, addressEntry, false); } public AddressEntry getFreshAddressEntry() { + return getFreshAddressEntry(true); + } + + public AddressEntry getFreshAddressEntry(boolean segwit) { AddressEntry.Context context = AddressEntry.Context.AVAILABLE; Optional addressEntry = getAddressEntryListAsImmutableList().stream() .filter(e -> context == e.getContext()) .filter(e -> isAddressUnused(e.getAddress())) + .filter(e -> Script.ScriptType.P2WPKH.equals(e.getAddress().getOutputScriptType()) == segwit) .findAny(); - return getOrCreateAddressEntry(context, addressEntry); + return getOrCreateAddressEntry(context, addressEntry, segwit); } public void recoverAddressEntry(String offerId, String address, AddressEntry.Context context) { @@ -618,18 +623,15 @@ public void recoverAddressEntry(String offerId, String address, AddressEntry.Con addressEntryList.swapAvailableToAddressEntryWithOfferId(addressEntry, context, offerId)); } - private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, Optional addressEntry) { + private AddressEntry getOrCreateAddressEntry(AddressEntry.Context context, Optional addressEntry, boolean segwit) { if (addressEntry.isPresent()) { return addressEntry.get(); } else { DeterministicKey key; - boolean segwit; - if (AddressEntry.Context.AVAILABLE.equals(context)) { + if (segwit) { key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2WPKH)); - segwit = true; } else { key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); - segwit = false; } AddressEntry entry = new AddressEntry(key, context, segwit); addressEntryList.addAddressEntry(entry); diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 3afb4f273af..32c2813bed9 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -992,6 +992,7 @@ funds.deposit.fundWallet=Fund your wallet funds.deposit.withdrawFromWallet=Send funds from wallet funds.deposit.amount=Amount in BTC (optional) funds.deposit.generateAddress=Generate new address +funds.deposit.generateAddressSegwit=Native segwit format (Bech32) funds.deposit.selectUnused=Please select an unused address from the table above rather than generating a new one. funds.withdrawal.arbitrationFee=Arbitration fee diff --git a/desktop/src/main/java/bisq/desktop/main/funds/deposit/DepositView.java b/desktop/src/main/java/bisq/desktop/main/funds/deposit/DepositView.java index 3d08c7528ce..1195c069369 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/deposit/DepositView.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/deposit/DepositView.java @@ -41,8 +41,12 @@ import bisq.common.UserThread; import bisq.common.app.DevEnv; +import bisq.common.config.Config; +import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; +import org.bitcoinj.core.NetworkParameters; +import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Transaction; import net.glxn.qrgen.QRCode; @@ -54,6 +58,7 @@ import javafx.fxml.FXML; import javafx.scene.control.Button; +import javafx.scene.control.CheckBox; import javafx.scene.control.TableCell; import javafx.scene.control.TableColumn; import javafx.scene.control.TableView; @@ -85,10 +90,7 @@ import org.jetbrains.annotations.NotNull; -import static bisq.desktop.util.FormBuilder.addAddressTextField; -import static bisq.desktop.util.FormBuilder.addButton; -import static bisq.desktop.util.FormBuilder.addInputTextField; -import static bisq.desktop.util.FormBuilder.addTitledGroupBg; +import static bisq.desktop.util.FormBuilder.*; @FxmlView public class DepositView extends ActivatableView { @@ -102,6 +104,7 @@ public class DepositView extends ActivatableView { private ImageView qrCodeImageView; private AddressTextField addressTextField; private Button generateNewAddressButton; + private CheckBox generateNewAddressSegwitCheckbox; private TitledGroupBg titledGroupBg; private InputTextField amountTextField; @@ -195,16 +198,26 @@ public void initialize() { addressTextField.setManaged(false); amountTextField.setManaged(false); + generateNewAddressSegwitCheckbox = addCheckBox(gridPane, ++gridRow, + Res.get("funds.deposit.generateAddressSegwit"), -20); + generateNewAddressSegwitCheckbox.setAllowIndeterminate(false); + generateNewAddressSegwitCheckbox.setSelected(true); + GridPane.setColumnIndex(generateNewAddressSegwitCheckbox, 0); + GridPane.setHalignment(generateNewAddressSegwitCheckbox, HPos.LEFT); + generateNewAddressButton = addButton(gridPane, ++gridRow, Res.get("funds.deposit.generateAddress"), -20); GridPane.setColumnIndex(generateNewAddressButton, 0); GridPane.setHalignment(generateNewAddressButton, HPos.LEFT); generateNewAddressButton.setOnAction(event -> { - boolean hasUnUsedAddress = observableList.stream().anyMatch(e -> e.getNumTxOutputs() == 0); + boolean segwit = generateNewAddressSegwitCheckbox.isSelected(); + NetworkParameters params = Config.baseCurrencyNetworkParameters(); + boolean hasUnUsedAddress = observableList.stream().anyMatch(e -> e.getNumTxOutputs() == 0 + && (Address.fromString(params, e.getAddressString()) instanceof SegwitAddress) == segwit); if (hasUnUsedAddress) { new Popup().warning(Res.get("funds.deposit.selectUnused")).show(); } else { - AddressEntry newSavingsAddressEntry = walletService.getFreshAddressEntry(); + AddressEntry newSavingsAddressEntry = walletService.getFreshAddressEntry(segwit); updateList(); observableList.stream() .filter(depositListItem -> depositListItem.getAddressString().equals(newSavingsAddressEntry.getAddressString())) From 1f3c585c350320be1547bb7b47963b385dad736d Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 17:41:11 -0300 Subject: [PATCH 12/30] Serialize tx without segwit This is for backwards compatibility with bisq nodes that have not upgraded to bitcoinj 0.15. --- core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d0fbd194607..0951706590e 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -1091,7 +1091,7 @@ private RawTransactionInput getRawInputFromTransactionInput(@NotNull Transaction checkNotNull(input.getValue(), "input.getValue() must not be null"); return new RawTransactionInput(input.getOutpoint().getIndex(), - input.getConnectedOutput().getParentTransaction().bitcoinSerialize(), + input.getConnectedOutput().getParentTransaction().bitcoinSerialize(false), input.getValue().value); } From 58afc00282d26fc0d8f92937b4fb955f31a269d4 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 20:17:49 -0300 Subject: [PATCH 13/30] Don't create an extra address at startup --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d190f37170c..8146bba6970 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -321,7 +321,7 @@ private Wallet createOrLoadWallet(boolean shouldReplayWallet, File walletFile, b wallet = loadWallet(shouldReplayWallet, walletFile, isBsqWallet); } else { wallet = createWallet(isBsqWallet); - wallet.freshReceiveKey(); + //wallet.freshReceiveKey(); // Currently the only way we can be sure that an extension is aware of its containing wallet is by // deserializing the extension (see WalletExtension#deserializeWalletExtension(Wallet, byte[])) From e2f74f025093ed2bfa71cf5d0b7922691f435484 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 28 Sep 2020 20:18:17 -0300 Subject: [PATCH 14/30] Don't create a wallet address when not needed --- core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6603342fbff..0e6e2e2b89b 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -985,7 +985,7 @@ public Transaction getFeeEstimationTransactionForMultipleAddresses(Set f counter++; fee = txFeeForWithdrawalPerByte.multiply(txSize); // We use a dummy address for the output - final String dummyReceiver = getFreshAddressEntry().getAddressString(); + final String dummyReceiver = LegacyAddress.fromKey(params, new ECKey()).toBase58(); SendRequest sendRequest = getSendRequestForMultipleAddresses(fromAddresses, dummyReceiver, amount, fee, null, aesKey); wallet.completeTx(sendRequest); tx = sendRequest.tx; From d1aeedd98b9eec683202b6210511fa5ce87cae0d Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 24 Sep 2020 17:44:09 -0300 Subject: [PATCH 15/30] Remove unused WalletService.findKeyFromPubKeyHash() --- core/src/main/java/bisq/core/btc/wallet/WalletService.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletService.java b/core/src/main/java/bisq/core/btc/wallet/WalletService.java index f17122ac3b3..8ad8e01db35 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletService.java @@ -606,11 +606,6 @@ public boolean checkAESKey(KeyParameter aesKey) { return wallet.checkAESKey(aesKey); } - @Nullable - public DeterministicKey findKeyFromPubKeyHash(byte[] pubKeyHash) { - return wallet.getActiveKeyChain().findKeyFromPubHash(pubKeyHash); - } - @Nullable public DeterministicKey findKeyFromPubKey(byte[] pubKey) { return (DeterministicKey) wallet.findKeyFromPubKey(pubKey); From 78a2a43d48cea65688e0946c8b8bac3ebac66fc3 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 14 Sep 2020 17:49:53 -0300 Subject: [PATCH 16/30] Remove unused import --- core/src/main/java/bisq/core/btc/model/AddressEntry.java | 1 - 1 file changed, 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 4e40c5d0a5d..5348950b391 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -26,7 +26,6 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.crypto.DeterministicKey; import org.bitcoinj.script.Script; From 01455d2b209cc49a4f3688add3e481422c1bb3a2 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 1 Oct 2020 11:54:20 -0300 Subject: [PATCH 17/30] Enable reusing unused AVAILABLE entries --- .../bisq/core/btc/wallet/BtcWalletService.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 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 0e6e2e2b89b..e638f5a2861 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -579,20 +579,20 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context if (addressEntry.isPresent()) { return addressEntry.get(); } else { -// Disable reusing unused AVAILABLE entries until segwit support in mandatory in Bisq // We try to use available and not yet used entries -// Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream() -// .filter(e -> AddressEntry.Context.AVAILABLE == e.getContext()) -// .filter(e -> isAddressUnused(e.getAddress())) -// .findAny(); -// if (emptyAvailableAddressEntry.isPresent()) { -// return addressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); -// } else { + Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream() + .filter(e -> AddressEntry.Context.AVAILABLE == e.getContext()) + .filter(e -> isAddressUnused(e.getAddress())) + .filter(e -> Script.ScriptType.P2PKH.equals(e.getAddress().getOutputScriptType())) + .findAny(); + if (emptyAvailableAddressEntry.isPresent()) { + return addressEntryList.swapAvailableToAddressEntryWithOfferId(emptyAvailableAddressEntry.get(), context, offerId); + } else { DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2PKH)); AddressEntry entry = new AddressEntry(key, context, offerId, false); addressEntryList.addAddressEntry(entry); return entry; -// } + } } } From 4a2c0ad75a8b49748b687f7171281d189b333d37 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 1 Oct 2020 13:15:02 -0300 Subject: [PATCH 18/30] Make codacy happy --- core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java | 2 +- core/src/main/java/bisq/core/btc/wallet/WalletService.java | 1 - 2 files changed, 1 insertion(+), 2 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 0951706590e..7422a62e0da 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -604,7 +604,7 @@ public Transaction takerSignsDepositTx(boolean takerIsSeller, TransactionInput transactionInput = makersDepositTx.getInputs().get(i); depositTx.addInput(getTransactionInput(depositTx, getMakersScriptSigProgram(transactionInput), buyerInputs.get(i))); if (!TransactionWitness.EMPTY.equals(transactionInput.getWitness())) { - depositTx.getInput(depositTx.getInputs().size()-1).setWitness(transactionInput.getWitness()); + depositTx.getInput(depositTx.getInputs().size() - 1).setWitness(transactionInput.getWitness()); } } diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletService.java b/core/src/main/java/bisq/core/btc/wallet/WalletService.java index 8ad8e01db35..84005978348 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletService.java @@ -17,7 +17,6 @@ package bisq.core.btc.wallet; -import bisq.core.btc.exceptions.SigningException; import bisq.core.btc.exceptions.TransactionVerificationException; import bisq.core.btc.exceptions.WalletException; import bisq.core.btc.listeners.AddressConfidenceListener; From 86ddd06e276f02cd07a137214b851666926acc69 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 15:08:19 -0300 Subject: [PATCH 19/30] Validate AddressEntry.segwit --- core/src/main/java/bisq/core/btc/model/AddressEntry.java | 4 ++++ 1 file changed, 4 insertions(+) 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..a3a0387be2a 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntry.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntry.java @@ -97,6 +97,10 @@ public AddressEntry(@NotNull DeterministicKey keyPair, Context context, @Nullable String offerId, boolean segwit) { + if (segwit && (!Context.AVAILABLE.equals(context) || offerId != null)) { + throw new IllegalArgumentException("Segwit addresses are only allowed for " + + "AVAILABLE entries without an offerId"); + } this.keyPair = keyPair; this.context = context; this.offerId = offerId; From b9e404f0e25a8d14fd57eecea1850e97f7f5cc65 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 16:50:14 -0300 Subject: [PATCH 20/30] Make it clear segwit is not used for the trade protocol yet. --- core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java | 1 + 1 file changed, 1 insertion(+) 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 e638f5a2861..3521903a68c 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -579,6 +579,7 @@ public AddressEntry getOrCreateAddressEntry(String offerId, AddressEntry.Context if (addressEntry.isPresent()) { return addressEntry.get(); } else { + // We still use non-segwit addresses for the trade protocol. // We try to use available and not yet used entries Optional emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream() .filter(e -> AddressEntry.Context.AVAILABLE == e.getContext()) From 5524ba3e97d98f7f4559625ee1e83307596f1efe Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:05:59 -0300 Subject: [PATCH 21/30] BtcWalletService.getFreshAddressEntry(): code clean up --- .../main/java/bisq/core/btc/wallet/BtcWalletService.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 3521903a68c..04e1cce087b 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -614,7 +614,12 @@ public AddressEntry getFreshAddressEntry(boolean segwit) { Optional addressEntry = getAddressEntryListAsImmutableList().stream() .filter(e -> context == e.getContext()) .filter(e -> isAddressUnused(e.getAddress())) - .filter(e -> Script.ScriptType.P2WPKH.equals(e.getAddress().getOutputScriptType()) == segwit) + .filter(e -> { + boolean isSegwitOutputScriptType = Script.ScriptType.P2WPKH.equals(e.getAddress().getOutputScriptType()); + // We need to ensure that we take only addressEntries which matches our segWit flag + boolean isMatchingOutputScriptType = isSegwitOutputScriptType == segwit; + return isMatchingOutputScriptType; + }) .findAny(); return getOrCreateAddressEntry(context, addressEntry, segwit); } From d1aaf3e4eb9770d6a551001428e2599aa0616f61 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:26:06 -0300 Subject: [PATCH 22/30] Construct dummy outputs with LegacyAddress --- .../main/java/bisq/core/btc/wallet/TradeWalletService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 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 7422a62e0da..b58c7c9f9e9 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -38,7 +38,6 @@ import org.bitcoinj.core.InsufficientMoneyException; import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.NetworkParameters; -import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Sha256Hash; import org.bitcoinj.core.SignatureDecodeException; import org.bitcoinj.core.Transaction; @@ -337,7 +336,7 @@ OUT[0] dummyOutputAmount (inputAmount - tx fee) Transaction dummyTX = new Transaction(params); // The output is just used to get the right inputs and change outputs, so we use an anonymous ECKey, as it will never be used for anything. // We don't care about fee calculation differences between the real tx and that dummy tx as we use a static tx fee. - TransactionOutput dummyOutput = new TransactionOutput(params, dummyTX, dummyOutputAmount, SegwitAddress.fromKey(params, new ECKey())); + TransactionOutput dummyOutput = new TransactionOutput(params, dummyTX, dummyOutputAmount, LegacyAddress.fromKey(params, new ECKey())); dummyTX.addOutput(dummyOutput); // Find the needed inputs to pay the output, optionally add 1 change output. @@ -456,7 +455,7 @@ private PreparedDepositTxAndMakerInputs makerCreatesDepositTx(boolean makerIsBuy // First we construct a dummy TX to get the inputs and outputs we want to use for the real deposit tx. // Similar to the way we did in the createTakerDepositTxInputs method. Transaction dummyTx = new Transaction(params); - TransactionOutput dummyOutput = new TransactionOutput(params, dummyTx, makerInputAmount, SegwitAddress.fromKey(params, new ECKey())); + TransactionOutput dummyOutput = new TransactionOutput(params, dummyTx, makerInputAmount, LegacyAddress.fromKey(params, new ECKey())); dummyTx.addOutput(dummyOutput); addAvailableInputsAndChangeOutputs(dummyTx, makerAddress, makerChangeAddress); // Normally we have only 1 input but we support multiple inputs if the user has paid in with several transactions. From 3554e190842d100d2fb9f111a9219fea75024231 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:38:31 -0300 Subject: [PATCH 23/30] setWitness(): Code clean up --- .../core/btc/wallet/TradeWalletService.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 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 b58c7c9f9e9..eb7b7485c10 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -600,11 +600,13 @@ public Transaction takerSignsDepositTx(boolean takerIsSeller, // Add buyer inputs and apply signature // We grab the signature from the makersDepositTx and apply it to the new tx input for (int i = 0; i < buyerInputs.size(); i++) { - TransactionInput transactionInput = makersDepositTx.getInputs().get(i); - depositTx.addInput(getTransactionInput(depositTx, getMakersScriptSigProgram(transactionInput), buyerInputs.get(i))); - if (!TransactionWitness.EMPTY.equals(transactionInput.getWitness())) { - depositTx.getInput(depositTx.getInputs().size() - 1).setWitness(transactionInput.getWitness()); + TransactionInput makersInput = makersDepositTx.getInputs().get(i); + byte[] makersScriptSigProgram = getMakersScriptSigProgram(makersInput); + TransactionInput input = getTransactionInput(depositTx, makersScriptSigProgram, buyerInputs.get(i)); + if (!TransactionWitness.EMPTY.equals(makersInput.getWitness())) { + input.setWitness(makersInput.getWitness()); } + depositTx.addInput(input); } // Add seller inputs @@ -666,12 +668,13 @@ public void sellerAsMakerFinalizesDepositTx(Transaction myDepositTx, // We add takers signature from his inputs and add it to out tx which was already signed earlier. for (int i = 0; i < numTakersInputs; i++) { - TransactionInput input = takersDepositTx.getInput(i); - Script scriptSig = input.getScriptSig(); - myDepositTx.getInput(i).setScriptSig(scriptSig); - TransactionWitness witness = input.getWitness(); + TransactionInput takersInput = takersDepositTx.getInput(i); + Script takersScriptSig = takersInput.getScriptSig(); + TransactionInput txInput = myDepositTx.getInput(i); + txInput.setScriptSig(takersScriptSig); + TransactionWitness witness = takersInput.getWitness(); if (!TransactionWitness.EMPTY.equals(witness)) { - myDepositTx.getInput(i).setWitness(witness); + txInput.setWitness(witness); } } From 499d7b7d35683b4f0747e9a674887c9329cc08c3 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:40:42 -0300 Subject: [PATCH 24/30] Use try-with-resources --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 8146bba6970..22aaae82fad 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -341,8 +341,7 @@ protected void setupAutoSave(Wallet wallet, File walletFile) { private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean isBsqWallet) throws Exception { Wallet wallet; - FileInputStream walletStream = new FileInputStream(walletFile); - try { + try (FileInputStream walletStream = new FileInputStream(walletFile)) { WalletExtension[] extArray = new WalletExtension[]{}; Protos.Wallet proto = WalletProtobufSerializer.parseToProto(walletStream); final WalletProtobufSerializer serializer; @@ -360,8 +359,6 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i .accountPath(new BisqKeyChainGroupStructure(isBsqWallet).accountPathFor(Script.ScriptType.P2WPKH)).build(); wallet.addAndActivateHDChain(nativeSegwitKeyChain); } - } finally { - walletStream.close(); } return wallet; } From 1d82c01670ba2943ef700d2e4116a0f649eb1b46 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:49:20 -0300 Subject: [PATCH 25/30] Improve error handling for P2WPKH --- .../bisq/core/btc/wallet/WalletService.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletService.java b/core/src/main/java/bisq/core/btc/wallet/WalletService.java index 84005978348..920df501e82 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletService.java @@ -327,16 +327,22 @@ public static void signTransactionInput(Wallet wallet, log.warn("No private key in keypair for input {}", index); } } else if (ScriptPattern.isP2WPKH(scriptPubKey)) { - // TODO: Consider using this alternative way to build the scriptCode (taken from bitcoinj master) - // Script scriptCode = ScriptBuilder.createP2PKHOutputScript(key); - Script scriptCode = new ScriptBuilder().data( - ScriptBuilder.createOutputScript(LegacyAddress.fromKey(tx.getParams(), key)).getProgram()) - .build(); - Coin value = txIn.getValue(); - TransactionSignature txSig = tx.calculateWitnessSignature(index, key, scriptCode, value, - Transaction.SigHash.ALL, false); - txIn.setScriptSig(ScriptBuilder.createEmpty()); - txIn.setWitness(TransactionWitness.redeemP2WPKH(txSig, key)); + try { + // TODO: Consider using this alternative way to build the scriptCode (taken from bitcoinj master) + // Script scriptCode = ScriptBuilder.createP2PKHOutputScript(key); + Script scriptCode = new ScriptBuilder().data( + ScriptBuilder.createOutputScript(LegacyAddress.fromKey(tx.getParams(), key)).getProgram()) + .build(); + Coin value = txIn.getValue(); + TransactionSignature txSig = tx.calculateWitnessSignature(index, key, scriptCode, value, + Transaction.SigHash.ALL, false); + txIn.setScriptSig(ScriptBuilder.createEmpty()); + txIn.setWitness(TransactionWitness.redeemP2WPKH(txSig, key)); + } catch (ECKey.KeyIsEncryptedException e1) { + throw e1; + } catch (ECKey.MissingPrivateKeyException e1) { + log.warn("No private key in keypair for input {}", index); + } } else { // log.error("Unexpected script type."); throw new RuntimeException("Unexpected script type."); From 694446c296466904aa748f45db78b73d854d7d1e Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Mon, 5 Oct 2020 17:58:07 -0300 Subject: [PATCH 26/30] Switch back to LegacyAddress for fee estimation --- core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 04e1cce087b..e21ce45b99a 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -35,7 +35,6 @@ import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; import org.bitcoinj.core.LegacyAddress; -import org.bitcoinj.core.SegwitAddress; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.core.TransactionInput; @@ -1020,7 +1019,7 @@ private boolean feeEstimationNotSatisfied(int counter, Transaction tx) { public int getEstimatedFeeTxSize(List outputValues, Coin txFee) throws InsufficientMoneyException, AddressFormatException { Transaction transaction = new Transaction(params); - Address dummyAddress = SegwitAddress.fromKey(params, new ECKey()); + Address dummyAddress = LegacyAddress.fromKey(params, new ECKey()); outputValues.forEach(outputValue -> transaction.addOutput(outputValue, dummyAddress)); SendRequest sendRequest = SendRequest.forTx(transaction); From a747e83211492a8d10ef4ce59d3c4cab99074948 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Wed, 7 Oct 2020 16:31:44 -0300 Subject: [PATCH 27/30] Fix add segwit keychain for encrypted wallet --- .../main/java/bisq/core/app/BisqSetup.java | 2 + .../bisq/core/btc/setup/WalletConfig.java | 78 ++++++++++++++----- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index 6027f7f892a..3660544f60d 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -389,6 +389,8 @@ private void initWallet() { if (requestWalletPasswordHandler != null) { requestWalletPasswordHandler.accept(aesKey -> { walletsManager.setAesKey(aesKey); + walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), + aesKey); if (preferences.isResyncSpvRequested()) { if (showFirstPopupIfResyncSPVRequestedHandler != null) showFirstPopupIfResyncSPVRequestedHandler.run(); 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 22aaae82fad..6bf36d82b48 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -27,6 +27,7 @@ import com.google.common.util.concurrent.*; import org.bitcoinj.core.listeners.*; import org.bitcoinj.core.*; +import org.bitcoinj.crypto.KeyCrypter; import org.bitcoinj.net.BlockingClientManager; import org.bitcoinj.net.discovery.*; import org.bitcoinj.script.Script; @@ -35,6 +36,11 @@ import com.runjva.sourceforge.jsocks.protocol.Socks5Proxy; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; + +import org.bouncycastle.crypto.params.KeyParameter; + import org.slf4j.*; import javax.annotation.*; @@ -103,6 +109,8 @@ public class WalletConfig extends AbstractIdleService { @Getter @Setter private int minBroadcastConnections; + @Getter + private BooleanProperty migratedWalletToSegwit = new SimpleBooleanProperty(false); /** * Creates a new WalletConfig, with a newly created {@link Context}. Files will be stored in the given directory. @@ -293,25 +301,34 @@ protected void startUp() throws Exception { vPeerGroup.addWallet(vBsqWallet); onSetupCompleted(); - Futures.addCallback((ListenableFuture) vPeerGroup.startAsync(), new FutureCallback() { - @Override - public void onSuccess(@Nullable Object result) { - //completeExtensionInitiations(vPeerGroup); - DownloadProgressTracker tracker = downloadListener == null ? new DownloadProgressTracker() : downloadListener; - vPeerGroup.startBlockChainDownload(tracker); - } - - @Override - public void onFailure(Throwable t) { - throw new RuntimeException(t); + if (migratedWalletToSegwit.get()) { + startPeerGroup(); + } else { + migratedWalletToSegwit.addListener((observable, oldValue, newValue) -> startPeerGroup()); + } - } - }, MoreExecutors.directExecutor()); } catch (BlockStoreException e) { throw new IOException(e); } } + private void startPeerGroup() { + Futures.addCallback((ListenableFuture) vPeerGroup.startAsync(), new FutureCallback() { + @Override + public void onSuccess(@Nullable Object result) { + //completeExtensionInitiations(vPeerGroup); + DownloadProgressTracker tracker = downloadListener == null ? new DownloadProgressTracker() : downloadListener; + vPeerGroup.startBlockChainDownload(tracker); + } + + @Override + public void onFailure(Throwable t) { + throw new RuntimeException(t); + + } + }, MoreExecutors.directExecutor()); + } + private Wallet createOrLoadWallet(boolean shouldReplayWallet, File walletFile, boolean isBsqWallet) throws Exception { Wallet wallet; @@ -351,13 +368,8 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i wallet = serializer.readWallet(params, extArray, proto); if (shouldReplayWallet) wallet.reset(); - if (!isBsqWallet && BisqKeyChainGroupStructure.BIP44_BTC_NON_SEGWIT_ACCOUNT_PATH.equals(wallet.getActiveKeyChain().getAccountPath())) { - // Btc wallet does not have a native segwit keychain, we should add one. - DeterministicSeed seed = wallet.getKeyChainSeed(); - DeterministicKeyChain nativeSegwitKeyChain = DeterministicKeyChain.builder().seed(seed) - .outputScriptType(Script.ScriptType.P2WPKH) - .accountPath(new BisqKeyChainGroupStructure(isBsqWallet).accountPathFor(Script.ScriptType.P2WPKH)).build(); - wallet.addAndActivateHDChain(nativeSegwitKeyChain); + if (!isBsqWallet) { + maybeAddSegwitKeychain(wallet, null); } } return wallet; @@ -491,4 +503,30 @@ public PeerGroup peerGroup() { public File directory() { return directory; } + + public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey) { + if (BisqKeyChainGroupStructure.BIP44_BTC_NON_SEGWIT_ACCOUNT_PATH.equals(wallet.getActiveKeyChain().getAccountPath())) { + if (wallet.isEncrypted() && aesKey == null) { + // wait for the aesKey to be set and this method to be invoked again. + return; + } + // Btc wallet does not have a native segwit keychain, we should add one. + DeterministicSeed seed = wallet.getKeyChainSeed(); + if (aesKey != null) { + // If wallet is encrypted, decrypt the seed. + KeyCrypter keyCrypter = wallet.getKeyCrypter(); + seed = seed.decrypt(keyCrypter, DeterministicKeyChain.DEFAULT_PASSPHRASE_FOR_MNEMONIC, aesKey); + } + DeterministicKeyChain nativeSegwitKeyChain = DeterministicKeyChain.builder().seed(seed) + .outputScriptType(Script.ScriptType.P2WPKH) + .accountPath(new BisqKeyChainGroupStructure(false).accountPathFor(Script.ScriptType.P2WPKH)).build(); + if (aesKey != null) { + // If wallet is encrypted, encrypt the new keychain. + KeyCrypter keyCrypter = wallet.getKeyCrypter(); + nativeSegwitKeyChain = nativeSegwitKeyChain.toEncrypted(keyCrypter, aesKey); + } + wallet.addAndActivateHDChain(nativeSegwitKeyChain); + } + migratedWalletToSegwit.set(true); + } } From 417daf56926c9f6a4c65dc0adfad473712ea0ddb Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 8 Oct 2020 15:56:49 -0300 Subject: [PATCH 28/30] Use bitcoinj 0.15.8 (commit a733034) --- 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 fe986909e55..56cdb1cc792 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ configure(subprojects) { ext { // in alphabetical order bcVersion = '1.63' - bitcoinjVersion = '44ddbdc' + bitcoinjVersion = 'a733034' 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 74049ae6376..c9b276b2948 100644 --- a/core/src/main/java/bisq/core/app/WalletAppSetup.java +++ b/core/src/main/java/bisq/core/app/WalletAppSetup.java @@ -105,7 +105,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, "44ddbdc"); + VersionMessage.BITCOINJ_VERSION, "a733034"); ObjectProperty walletServiceException = new SimpleObjectProperty<>(); btcInfoBinding = EasyBind.combine(walletsSetup.downloadPercentageProperty(), diff --git a/gradle/witness/gradle-witness.gradle b/gradle/witness/gradle-witness.gradle index 7ca83933227..f73dba9b81b 100644 --- a/gradle/witness/gradle-witness.gradle +++ b/gradle/witness/gradle-witness.gradle @@ -20,7 +20,7 @@ dependencyVerification { 'com.fasterxml.jackson.core:jackson-core:39a74610521d7fb9eb3f437bb8739bbf47f6435be12d17bf954c731a0c6352bb', 'com.fasterxml.jackson.core:jackson-databind:fcf3c2b0c332f5f54604f7e27fa7ee502378a2cc5df6a944bbfae391872c32ff', 'com.github.JesusMcCloud:jtorctl:389d61b1b5a85eb2f23c582c3913ede49f80c9f2b553e4762382c836270e57e5', - 'com.github.bisq-network:bitcoinj:85d609e9bbaa93de0a9ca1ab436f578c14f7cfa1876b50878046d9f624b48a6b', + 'com.github.bisq-network:bitcoinj:b8b6e4b8010f2b8d4aac7141c0809dea6d102c3ff3c06ceba78c2626d531b0af', 'com.github.cd2357.netlayer:tor.external:7c70846d36465279c2664f147a0f2d47202c5d67c6a2075225194779c3fbe122', 'com.github.cd2357.netlayer:tor.native:84b449191d535a3c2187f7f7f3bb9bcb7d1097f07c6bf8c4f2b3331c20107d9a', 'com.github.cd2357.netlayer:tor:ff92e4a7b59d1b480e0427fcfcf3f82a6fd69be68eec91c6360774d599e3c2e0', From 87da2ae349cee0d7733adbfbe290cc38dfc0e657 Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 8 Oct 2020 16:22:52 -0300 Subject: [PATCH 29/30] Do a backup of the wallet before segwit migration --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 9 +++++++++ core/src/main/java/bisq/core/btc/setup/WalletsSetup.java | 9 +++++++++ 2 files changed, 18 insertions(+) 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 6bf36d82b48..b7042e1c740 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -22,6 +22,7 @@ import bisq.core.btc.wallet.BisqRiskAnalysis; import bisq.common.config.Config; +import bisq.common.file.FileUtil; import com.google.common.io.Closeables; import com.google.common.util.concurrent.*; @@ -510,6 +511,14 @@ public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey) { // wait for the aesKey to be set and this method to be invoked again. return; } + // Do a backup of the wallet + File backup = new File(directory, WalletsSetup.PRE_SEGWIT_WALLET_BACKUP); + try { + FileUtil.copyFile(new File(directory, "bisq_BTC.wallet"), backup); + } catch (IOException e) { + log.error(e.toString(), e); + } + // Btc wallet does not have a native segwit keychain, we should add one. DeterministicSeed seed = wallet.getKeyChainSeed(); if (aesKey != null) { diff --git a/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java b/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java index 650f9297853..9b1ce934186 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java @@ -109,6 +109,8 @@ @Slf4j public class WalletsSetup { + public static final String PRE_SEGWIT_WALLET_BACKUP = "pre_segwit_bisq_BTC.wallet.backup"; + @Getter public final BooleanProperty walletsSetupFailed = new SimpleBooleanProperty(); @@ -421,6 +423,13 @@ public void clearBackups() { log.error("Could not delete directory " + e.getMessage()); e.printStackTrace(); } + + File segwitBackup = new File(walletDir, PRE_SEGWIT_WALLET_BACKUP); + try { + FileUtil.deleteFileIfExists(segwitBackup); + } catch (IOException e) { + log.error(e.toString(), e); + } } From 261e0ec714027be657ba7d1125ac65bd81b0135f Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Thu, 8 Oct 2020 17:26:24 -0300 Subject: [PATCH 30/30] Check migratedWalletToSegwit is true --- core/src/main/java/bisq/core/btc/setup/WalletConfig.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 b7042e1c740..fc8c6a09506 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -305,7 +305,11 @@ protected void startUp() throws Exception { if (migratedWalletToSegwit.get()) { startPeerGroup(); } else { - migratedWalletToSegwit.addListener((observable, oldValue, newValue) -> startPeerGroup()); + migratedWalletToSegwit.addListener((observable, oldValue, newValue) -> { + if (newValue) { + startPeerGroup(); + } + }); } } catch (BlockStoreException e) {