From ce73fb85f381e4a30a4cfb956a21887532a211db Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Wed, 20 Jan 2021 11:27:18 +0000 Subject: [PATCH 1/4] Code cleanup: Simplify Optional stream processing Use flatMap(Optional::stream) instead of filter(..isPresent).map(..get) and avoid a redundantly nested Optional in OpReturnType. Also replace some unnecessary stream().forEach(..) invocations on lists in BtcWalletService, as forEach is already part of the List interface. --- .../bisq/core/btc/wallet/BtcWalletService.java | 16 +++++++--------- .../core/dao/governance/period/CycleService.java | 3 +-- .../dao/state/model/blockchain/OpReturnType.java | 4 +--- 3 files changed, 9 insertions(+), 14 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 76752a87f61..bef6378b794 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -523,10 +523,10 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, } Transaction tx = new Transaction(params); - preparedBsqTxInputs.stream().forEach(tx::addInput); + preparedBsqTxInputs.forEach(tx::addInput); if (forcedChangeValue.isZero()) { - preparedBsqTxOutputs.stream().forEach(tx::addOutput); + preparedBsqTxOutputs.forEach(tx::addOutput); } else { //TODO test that case checkArgument(preparedBsqTxOutputs.size() == 0, "preparedBsqTxOutputs.size must be null in that code branch"); @@ -598,10 +598,10 @@ private Tuple2 getNumInputs(Transaction tx) { } else if (ScriptPattern.isP2WPKH(connectedOutput.getScriptPubKey())) { numSegwitInputs++; } else { - throw new IllegalArgumentException("Inputs should spend a P2PKH, P2PK or P2WPKH ouput"); + throw new IllegalArgumentException("Inputs should spend a P2PKH, P2PK or P2WPKH output"); } } - return new Tuple2(numLegacyInputs, numSegwitInputs); + return new Tuple2<>(numLegacyInputs, numSegwitInputs); } @@ -871,7 +871,7 @@ public void doubleSpendTransaction(String txId, Runnable resultHandler, ErrorMes log.debug("txToDoubleSpend no. of inputs " + txToDoubleSpend.getInputs().size()); Transaction newTransaction = new Transaction(params); - txToDoubleSpend.getInputs().stream().forEach(input -> { + txToDoubleSpend.getInputs().forEach(input -> { final TransactionOutput connectedOutput = input.getConnectedOutput(); if (connectedOutput != null && connectedOutput.isMine(wallet) && @@ -1073,8 +1073,7 @@ public Transaction getFeeEstimationTransactionForMultipleAddresses(Set f addressEntryOptional = findAddressEntry(address, AddressEntry.Context.ARBITRATOR); return addressEntryOptional; }) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(Collectors.toSet()); if (addressEntries.isEmpty()) throw new AddressEntryException("No Addresses for withdraw found in our wallet"); @@ -1255,8 +1254,7 @@ private SendRequest getSendRequestForMultipleAddresses(Set fromAddresses addressEntryOptional = findAddressEntry(address, AddressEntry.Context.ARBITRATOR); return addressEntryOptional; }) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(Collectors.toSet()); if (addressEntries.isEmpty()) throw new AddressEntryException("No Addresses for withdraw found in our wallet"); diff --git a/core/src/main/java/bisq/core/dao/governance/period/CycleService.java b/core/src/main/java/bisq/core/dao/governance/period/CycleService.java index acc4cfa0313..a52fb127af2 100644 --- a/core/src/main/java/bisq/core/dao/governance/period/CycleService.java +++ b/core/src/main/java/bisq/core/dao/governance/period/CycleService.java @@ -134,8 +134,7 @@ private Cycle getFirstCycle() { // We add the default values from the Param enum to our StateChangeEvent list. List daoPhasesWithDefaultDuration = Arrays.stream(DaoPhase.Phase.values()) .map(this::getPhaseWithDefaultDuration) - .filter(Optional::isPresent) - .map(Optional::get) + .flatMap(Optional::stream) .collect(Collectors.toList()); return new Cycle(genesisBlockHeight, ImmutableList.copyOf(daoPhasesWithDefaultDuration)); } diff --git a/core/src/main/java/bisq/core/dao/state/model/blockchain/OpReturnType.java b/core/src/main/java/bisq/core/dao/state/model/blockchain/OpReturnType.java index 812288a6844..d2add05a7a9 100644 --- a/core/src/main/java/bisq/core/dao/state/model/blockchain/OpReturnType.java +++ b/core/src/main/java/bisq/core/dao/state/model/blockchain/OpReturnType.java @@ -51,8 +51,6 @@ public enum OpReturnType implements ImmutableDaoStateModel { public static Optional getOpReturnType(byte type) { return Arrays.stream(OpReturnType.values()) .filter(opReturnType -> opReturnType.type == type) - .map(Optional::of) - .findAny() - .orElse(Optional.empty()); + .findAny(); } } From 347042974669b0f8728df5270e0c566afc1236cb Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Thu, 21 Jan 2021 23:57:57 +0000 Subject: [PATCH 2/4] Allow use of bech32 BSQ addresses Remove the restriction to base58 (P2SH & P2PKH) addresses when parsing, formatting & validating BSQ addresses, by replacing o.b.c.LegacyAddress with its superclass o.b.c.Address throughout. Also remove restriction to LegacyAddress in BsqTxListItem and BsqTransfer(Service|Model). The bech32 BSQ addresses follow the same format as the old base58 BSQ addresses, namely 'B' + . --- .../apitest/method/wallet/BsqWalletTest.java | 4 ++-- assets/src/main/java/bisq/asset/coins/BSQ.java | 6 +++--- .../core/api/CorePaymentAccountsService.java | 2 +- .../java/bisq/core/api/CoreWalletsService.java | 17 ++++++++--------- .../bisq/core/btc/model/BsqTransferModel.java | 6 +++--- .../core/btc/wallet/BsqTransferService.java | 4 ++-- .../bisq/core/btc/wallet/BsqWalletService.java | 17 ++++++----------- .../governance/proposal/IssuanceProposal.java | 12 +++++++++++- .../model/governance/CompensationProposal.java | 11 +---------- .../model/governance/ReimbursementProposal.java | 11 +---------- .../java/bisq/core/util/coin/BsqFormatter.java | 10 +++++----- .../main/dao/wallet/tx/BsqTxListItem.java | 11 ++++------- .../util/validation/BsqAddressValidator.java | 2 +- 13 files changed, 48 insertions(+), 65 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java b/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java index 6be9dd66543..6c5ce8ec22a 100644 --- a/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java +++ b/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java @@ -2,7 +2,7 @@ import bisq.proto.grpc.BsqBalanceInfo; -import org.bitcoinj.core.LegacyAddress; +import org.bitcoinj.core.Address; import org.bitcoinj.core.NetworkParameters; import lombok.extern.slf4j.Slf4j; @@ -64,7 +64,7 @@ public void testGetUnusedBsqAddress() { assertFalse(address.isEmpty()); assertTrue(address.startsWith("B")); - NetworkParameters networkParameters = LegacyAddress.getParametersFromAddress(address.substring(1)); + NetworkParameters networkParameters = Address.fromString(null, address.substring(1)).getParameters(); String addressNetwork = networkParameters.getPaymentProtocolId(); assertNotEquals(PAYMENT_PROTOCOL_ID_MAINNET, addressNetwork); // TODO Fix bug causing the regtest bsq address network to be evaluated as 'testnet' here. diff --git a/assets/src/main/java/bisq/asset/coins/BSQ.java b/assets/src/main/java/bisq/asset/coins/BSQ.java index f769a582e38..4f9b6d7a3b6 100644 --- a/assets/src/main/java/bisq/asset/coins/BSQ.java +++ b/assets/src/main/java/bisq/asset/coins/BSQ.java @@ -18,7 +18,7 @@ package bisq.asset.coins; import bisq.asset.AddressValidationResult; -import bisq.asset.Base58AddressValidator; +import bisq.asset.BitcoinAddressValidator; import bisq.asset.Coin; import org.bitcoinj.core.NetworkParameters; @@ -57,7 +57,7 @@ public Regtest() { } - public static class BSQAddressValidator extends Base58AddressValidator { + public static class BSQAddressValidator extends BitcoinAddressValidator { public BSQAddressValidator(NetworkParameters networkParameters) { super(networkParameters); @@ -68,7 +68,7 @@ public AddressValidationResult validate(String address) { if (!address.startsWith("B")) return AddressValidationResult.invalidAddress("BSQ address must start with 'B'"); - String addressAsBtc = address.substring(1, address.length()); + String addressAsBtc = address.substring(1); return super.validate(addressAsBtc); } diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index 0843e20ab76..220205fbee3 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -108,7 +108,7 @@ PaymentAccount createCryptoCurrencyPaymentAccount(String accountName, throw new IllegalArgumentException("api does not currently support " + currencyCode + " accounts"); // Validate the BSQ address string but ignore the return value. - coreWalletsService.getValidBsqLegacyAddress(address); + coreWalletsService.getValidBsqAddress(address); var cryptoCurrencyAccount = tradeInstant ? (InstantCryptoCurrencyAccount) PaymentAccountFactory.getPaymentAccount(PaymentMethod.BLOCK_CHAINS_INSTANT) diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 4af8c3dd12d..78d71ccb082 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -51,7 +51,6 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; import org.bitcoinj.core.InsufficientMoneyException; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; @@ -226,7 +225,7 @@ String getUnusedBsqAddress() { return bsqWalletService.getUnusedBsqAddressAsString(); } - void sendBsq(String address, + void sendBsq(String addressStr, String amount, String txFeeRate, TxBroadcaster.Callback callback) { @@ -234,10 +233,10 @@ void sendBsq(String address, verifyEncryptedWalletIsUnlocked(); try { - LegacyAddress legacyAddress = getValidBsqLegacyAddress(address); + Address address = getValidBsqAddress(addressStr); Coin receiverAmount = getValidTransferAmount(amount, bsqFormatter); Coin txFeePerVbyte = getTxFeeRateFromParamOrPreferenceOrFeeService(txFeeRate); - BsqTransferModel model = bsqTransferService.getBsqTransferModel(legacyAddress, + BsqTransferModel model = bsqTransferService.getBsqTransferModel(address, receiverAmount, txFeePerVbyte); log.info("Sending {} BSQ to {} with tx fee rate {} sats/byte.", @@ -313,7 +312,7 @@ void sendBtc(String address, } boolean verifyBsqSentToAddress(String address, String amount) { - Address receiverAddress = getValidBsqLegacyAddress(address); + Address receiverAddress = getValidBsqAddress(address); NetworkParameters networkParameters = getNetworkParameters(); Predicate isTxOutputAddressMatch = (txOut) -> txOut.getScriptPubKey().getToAddress(networkParameters).equals(receiverAddress); @@ -553,12 +552,12 @@ void verifyApplicationIsFullyInitialized() { throw new IllegalStateException("server is not fully initialized"); } - // Returns a LegacyAddress for the string, or a RuntimeException if invalid. - LegacyAddress getValidBsqLegacyAddress(String address) { + // Returns an Address for the string, or a RuntimeException if invalid. + Address getValidBsqAddress(String address) { try { return bsqFormatter.getAddressFromBsqAddress(address); - } catch (Throwable t) { - log.error("", t); + } catch (RuntimeException e) { + log.error("", e); throw new IllegalStateException(format("%s is not a valid bsq address", address)); } } diff --git a/core/src/main/java/bisq/core/btc/model/BsqTransferModel.java b/core/src/main/java/bisq/core/btc/model/BsqTransferModel.java index 4ee77f24aff..3b31991ef75 100644 --- a/core/src/main/java/bisq/core/btc/model/BsqTransferModel.java +++ b/core/src/main/java/bisq/core/btc/model/BsqTransferModel.java @@ -2,8 +2,8 @@ import bisq.core.dao.state.model.blockchain.TxType; +import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.Transaction; import lombok.Getter; @@ -11,7 +11,7 @@ @Getter public final class BsqTransferModel { - private final LegacyAddress receiverAddress; + private final Address receiverAddress; private final Coin receiverAmount; private final Transaction preparedSendTx; private final Transaction txWithBtcFee; @@ -20,7 +20,7 @@ public final class BsqTransferModel { private final int txSize; private final TxType txType; - public BsqTransferModel(LegacyAddress receiverAddress, + public BsqTransferModel(Address receiverAddress, Coin receiverAmount, Transaction preparedSendTx, Transaction txWithBtcFee, diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java index 4558b0acf74..5639004c70f 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java @@ -5,9 +5,9 @@ import bisq.core.btc.exceptions.WalletException; import bisq.core.btc.model.BsqTransferModel; +import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; import org.bitcoinj.core.InsufficientMoneyException; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.Transaction; import javax.inject.Inject; @@ -32,7 +32,7 @@ public BsqTransferService(WalletsManager walletsManager, this.btcWalletService = btcWalletService; } - public BsqTransferModel getBsqTransferModel(LegacyAddress address, + public BsqTransferModel getBsqTransferModel(Address address, Coin receiverAmount, Coin txFeePerVbyte) throws TransactionVerificationException, 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 e8e1afa98cc..945ecd08ed1 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -527,7 +527,7 @@ public void commitTx(Transaction tx, TxType txType) { public Transaction getPreparedSendBsqTx(String receiverAddress, Coin receiverAmount) throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException, BsqChangeBelowDustException { - return getPreparedSendTx(receiverAddress, receiverAmount, bsqCoinSelector, false); + return getPreparedSendTx(receiverAddress, receiverAmount, bsqCoinSelector); } public Transaction getPreparedSendBsqTx(String receiverAddress, @@ -538,7 +538,7 @@ public Transaction getPreparedSendBsqTx(String receiverAddress, if (utxoCandidates != null) { bsqCoinSelector.setUtxoCandidates(utxoCandidates); } - return getPreparedSendTx(receiverAddress, receiverAmount, bsqCoinSelector, false); + return getPreparedSendTx(receiverAddress, receiverAmount, bsqCoinSelector); } /////////////////////////////////////////////////////////////////////////////////////////// @@ -548,7 +548,7 @@ public Transaction getPreparedSendBsqTx(String receiverAddress, public Transaction getPreparedSendBtcTx(String receiverAddress, Coin receiverAmount) throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException, BsqChangeBelowDustException { - return getPreparedSendTx(receiverAddress, receiverAmount, nonBsqCoinSelector, true); + return getPreparedSendTx(receiverAddress, receiverAmount, nonBsqCoinSelector); } public Transaction getPreparedSendBtcTx(String receiverAddress, @@ -559,21 +559,16 @@ public Transaction getPreparedSendBtcTx(String receiverAddress, if (utxoCandidates != null) { nonBsqCoinSelector.setUtxoCandidates(utxoCandidates); } - return getPreparedSendTx(receiverAddress, receiverAmount, nonBsqCoinSelector, true); + return getPreparedSendTx(receiverAddress, receiverAmount, nonBsqCoinSelector); } - private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmount, CoinSelector coinSelector, - boolean allowSegwitOuput) + private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmount, CoinSelector coinSelector) throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException, BsqChangeBelowDustException { daoKillSwitch.assertDaoIsNotDisabled(); Transaction tx = new Transaction(params); checkArgument(Restrictions.isAboveDust(receiverAmount), "The amount is too low (dust limit)."); - if (allowSegwitOuput) { - tx.addOutput(receiverAmount, Address.fromString(params, receiverAddress)); - } else { - tx.addOutput(receiverAmount, LegacyAddress.fromBase58(params, receiverAddress)); - } + tx.addOutput(receiverAmount, Address.fromString(params, receiverAddress)); SendRequest sendRequest = SendRequest.forTx(tx); sendRequest.fee = Coin.ZERO; sendRequest.feePerKb = Coin.ZERO; diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/IssuanceProposal.java b/core/src/main/java/bisq/core/dao/governance/proposal/IssuanceProposal.java index a466b999651..926ce6d48fd 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/IssuanceProposal.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/IssuanceProposal.java @@ -17,10 +17,14 @@ package bisq.core.dao.governance.proposal; +import bisq.common.config.Config; + +import org.bitcoinj.core.Address; +import org.bitcoinj.core.AddressFormatException; import org.bitcoinj.core.Coin; /** - * Marker interface for proposals which can lead to new BSQ issuance + * Interface for proposals which can lead to new BSQ issuance */ public interface IssuanceProposal { Coin getRequestedBsq(); @@ -28,4 +32,10 @@ public interface IssuanceProposal { String getBsqAddress(); String getTxId(); + + default Address getAddress() throws AddressFormatException { + // Remove leading 'B' + String underlyingBtcAddress = getBsqAddress().substring(1); + return Address.fromString(Config.baseCurrencyNetworkParameters(), underlyingBtcAddress); + } } diff --git a/core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java b/core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java index d23249b6c4f..4ed48d694b9 100644 --- a/core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java +++ b/core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java @@ -24,12 +24,9 @@ import bisq.core.dao.state.model.blockchain.TxType; import bisq.common.app.Version; -import bisq.common.config.Config; import bisq.common.util.CollectionUtils; -import org.bitcoinj.core.AddressFormatException; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import java.util.Date; import java.util.Map; @@ -113,17 +110,11 @@ public static CompensationProposal fromProto(protobuf.Proposal proto) { // Getters /////////////////////////////////////////////////////////////////////////////////////////// + @Override public Coin getRequestedBsq() { return Coin.valueOf(requestedBsq); } - public LegacyAddress getAddress() throws AddressFormatException { - // Remove leading 'B' - String underlyingBtcAddress = bsqAddress.substring(1, bsqAddress.length()); - return LegacyAddress.fromBase58(Config.baseCurrencyNetworkParameters(), underlyingBtcAddress); - } - - @Override public ProposalType getType() { return ProposalType.COMPENSATION_REQUEST; diff --git a/core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java b/core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java index fb83ab23d51..4e7d63f5c8d 100644 --- a/core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java +++ b/core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java @@ -24,12 +24,9 @@ import bisq.core.dao.state.model.blockchain.TxType; import bisq.common.app.Version; -import bisq.common.config.Config; import bisq.common.util.CollectionUtils; -import org.bitcoinj.core.AddressFormatException; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import java.util.Date; import java.util.Map; @@ -113,17 +110,11 @@ public static ReimbursementProposal fromProto(protobuf.Proposal proto) { // Getters /////////////////////////////////////////////////////////////////////////////////////////// + @Override public Coin getRequestedBsq() { return Coin.valueOf(requestedBsq); } - public LegacyAddress getAddress() throws AddressFormatException { - // Remove leading 'B' - String underlyingBtcAddress = bsqAddress.substring(1, bsqAddress.length()); - return LegacyAddress.fromBase58(Config.baseCurrencyNetworkParameters(), underlyingBtcAddress); - } - - @Override public ProposalType getType() { return ProposalType.REIMBURSEMENT_REQUEST; diff --git a/core/src/main/java/bisq/core/util/coin/BsqFormatter.java b/core/src/main/java/bisq/core/util/coin/BsqFormatter.java index ab476097bed..cd0accdb972 100644 --- a/core/src/main/java/bisq/core/util/coin/BsqFormatter.java +++ b/core/src/main/java/bisq/core/util/coin/BsqFormatter.java @@ -31,9 +31,9 @@ import bisq.common.config.Config; import bisq.common.util.MathUtils; +import org.bitcoinj.core.Address; import org.bitcoinj.core.AddressFormatException; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.utils.MonetaryFormat; import javax.inject.Inject; @@ -94,7 +94,7 @@ private void switchLocale(Locale locale) { * Returns the base-58 encoded String representation of this * object, including version and checksum bytes. */ - public String getBsqAddressStringFromAddress(LegacyAddress address) { + public String getBsqAddressStringFromAddress(Address address) { final String addressString = address.toString(); if (useBsqAddressFormat) return prefix + addressString; @@ -103,13 +103,13 @@ public String getBsqAddressStringFromAddress(LegacyAddress address) { } - public LegacyAddress getAddressFromBsqAddress(String encoded) { + public Address getAddressFromBsqAddress(String encoded) { String maybeUpdatedEncoded = encoded; if (useBsqAddressFormat) - maybeUpdatedEncoded = encoded.substring(prefix.length(), encoded.length()); + maybeUpdatedEncoded = encoded.substring(prefix.length()); try { - return LegacyAddress.fromBase58(Config.baseCurrencyNetworkParameters(), maybeUpdatedEncoded); + return Address.fromString(Config.baseCurrencyNetworkParameters(), maybeUpdatedEncoded); } catch (AddressFormatException e) { throw new RuntimeException(e); } diff --git a/desktop/src/main/java/bisq/desktop/main/dao/wallet/tx/BsqTxListItem.java b/desktop/src/main/java/bisq/desktop/main/dao/wallet/tx/BsqTxListItem.java index 43d0b2a9b4a..a3d2bdd7834 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/wallet/tx/BsqTxListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/wallet/tx/BsqTxListItem.java @@ -30,7 +30,6 @@ import org.bitcoinj.core.Address; import org.bitcoinj.core.Coin; -import org.bitcoinj.core.LegacyAddress; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionOutput; @@ -103,10 +102,9 @@ class BsqTxListItem extends TxConfidenceListItem { WalletService.isOutputScriptConvertibleToAddress(output)) { // We don't support send txs with multiple outputs to multiple receivers, so we can // assume that only one output is not from our own wallets. - // We ignore segwit outputs Address addressFromOutput = WalletService.getAddressFromOutput(output); - if (addressFromOutput instanceof LegacyAddress) { - sendToAddress = bsqFormatter.getBsqAddressStringFromAddress((LegacyAddress) addressFromOutput); + if (addressFromOutput != null) { + sendToAddress = bsqFormatter.getBsqAddressStringFromAddress(addressFromOutput); break; } } @@ -118,9 +116,8 @@ class BsqTxListItem extends TxConfidenceListItem { for (TransactionOutput output : transaction.getOutputs()) { if (WalletService.isOutputScriptConvertibleToAddress(output)) { Address addressFromOutput = WalletService.getAddressFromOutput(output); - // We ignore segwit outputs - if (addressFromOutput instanceof LegacyAddress) { - receivedWithAddress = bsqFormatter.getBsqAddressStringFromAddress((LegacyAddress) addressFromOutput); + if (addressFromOutput != null) { + receivedWithAddress = bsqFormatter.getBsqAddressStringFromAddress(addressFromOutput); break; } } diff --git a/desktop/src/main/java/bisq/desktop/util/validation/BsqAddressValidator.java b/desktop/src/main/java/bisq/desktop/util/validation/BsqAddressValidator.java index c4f441b5f0d..b8069684026 100644 --- a/desktop/src/main/java/bisq/desktop/util/validation/BsqAddressValidator.java +++ b/desktop/src/main/java/bisq/desktop/util/validation/BsqAddressValidator.java @@ -46,7 +46,7 @@ private ValidationResult validateBsqAddress(String input) { try { bsqFormatter.getAddressFromBsqAddress(input); return new ValidationResult(true); - } catch (Throwable e) { + } catch (RuntimeException e) { return new ValidationResult(false, Res.get("validation.bsq.invalidFormat")); } } From 5f1e32a2265f600dd65cb2111a57355cb75db41d Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 23 Jan 2021 12:49:38 +0000 Subject: [PATCH 3/4] Perform segwit BSQ wallet migration upon startup Uncomment & enable the m/44'/142'/1' native segwit BSQ account path and add code to migrate the user's BSQ wallet to segwit upon startup, along the same lines as the existing BTC wallet segwit migration logic. That is, set P2WPKH as the default output type, add a native segwit key chain (set to active) to the BSQ wallet and back up the old 'bisq_BSQ.wallet' file to 'pre_segwit_bisq_BSQ.wallet.backup'. Also filter out legacy addresses coming from the original keychain from BsqWalletService.get(Unused|Change)Address. --- .../bisq/core/api/CoreWalletsService.java | 3 +- .../main/java/bisq/core/app/BisqSetup.java | 4 +- .../btc/setup/BisqKeyChainGroupStructure.java | 16 ++++---- .../bisq/core/btc/setup/WalletConfig.java | 39 +++++++++++++------ .../bisq/core/btc/setup/WalletsSetup.java | 17 ++++---- .../core/btc/wallet/BsqWalletService.java | 9 +++-- 6 files changed, 54 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 78d71ccb082..4ab6ff282a8 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -581,7 +581,8 @@ private void maybeSetWalletsManagerKey() { if (btcWalletService.getAesKey() == null || bsqWalletService.getAesKey() == null) { KeyParameter aesKey = new KeyParameter(tempAesKey.getKey()); walletsManager.setAesKey(aesKey); - walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), aesKey); + walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), aesKey, false); + walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().bsqWallet(), aesKey, true); } } diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index de6db2c2ba9..d83f2af6145 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -424,7 +424,9 @@ private void initWallet() { requestWalletPasswordHandler.accept(aesKey -> { walletsManager.setAesKey(aesKey); walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), - aesKey); + aesKey, false); + walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().bsqWallet(), + aesKey, true); if (getResyncSpvSemaphore()) { if (showFirstPopupIfResyncSPVRequestedHandler != null) showFirstPopupIfResyncSPVRequestedHandler.run(); 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 66ec8b7cea2..a1a23b49e00 100644 --- a/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java +++ b/core/src/main/java/bisq/core/btc/setup/BisqKeyChainGroupStructure.java @@ -47,15 +47,14 @@ public class BisqKeyChainGroupStructure implements KeyChainGroupStructure { new ChildNumber(142, true), ChildNumber.ZERO_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); + 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; + private final boolean isBsqWallet; - public BisqKeyChainGroupStructure (boolean isBsqWallet) { + public BisqKeyChainGroupStructure(boolean isBsqWallet) { this.isBsqWallet = isBsqWallet; } @@ -72,8 +71,7 @@ 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; - throw new IllegalArgumentException(outputScriptType.toString()); + return BIP44_BSQ_SEGWIT_ACCOUNT_PATH; else throw new IllegalArgumentException(outputScriptType.toString()); } 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 bd260c13d8b..afa52f8482a 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletConfig.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletConfig.java @@ -57,6 +57,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; +import javafx.beans.binding.BooleanExpression; import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; @@ -143,7 +144,11 @@ public class WalletConfig extends AbstractIdleService { @Setter private int minBroadcastConnections; @Getter - private BooleanProperty migratedWalletToSegwit = new SimpleBooleanProperty(false); + private BooleanProperty migratedWalletToBtcSegwit = new SimpleBooleanProperty(false); + @Getter + private BooleanProperty migratedWalletToBsqSegwit = new SimpleBooleanProperty(false); + @Getter + private BooleanExpression migratedWalletToSegwit = migratedWalletToBtcSegwit.and(migratedWalletToBsqSegwit); /** * Creates a new WalletConfig, with a newly created {@link Context}. Files will be stored in the given directory. @@ -406,15 +411,13 @@ private Wallet loadWallet(boolean shouldReplayWallet, File walletFile, boolean i wallet = serializer.readWallet(params, extArray, proto); if (shouldReplayWallet) wallet.reset(); - if (!isBsqWallet) { - maybeAddSegwitKeychain(wallet, null); - } + maybeAddSegwitKeychain(wallet, null, isBsqWallet); } return wallet; } protected Wallet createWallet(boolean isBsqWallet) { - Script.ScriptType preferredOutputScriptType = isBsqWallet ? Script.ScriptType.P2PKH : Script.ScriptType.P2WPKH; + Script.ScriptType preferredOutputScriptType = Script.ScriptType.P2WPKH; KeyChainGroupStructure structure = new BisqKeyChainGroupStructure(isBsqWallet); KeyChainGroup.Builder kcgBuilder = KeyChainGroup.builder(params, structure); if (restoreFromSeed != null) { @@ -545,21 +548,29 @@ public File directory() { return directory; } - public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey) { - if (BisqKeyChainGroupStructure.BIP44_BTC_NON_SEGWIT_ACCOUNT_PATH.equals(wallet.getActiveKeyChain().getAccountPath())) { + public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey, boolean isBsqWallet) { + var nonSegwitAccountPath = isBsqWallet + ? BisqKeyChainGroupStructure.BIP44_BSQ_NON_SEGWIT_ACCOUNT_PATH + : BisqKeyChainGroupStructure.BIP44_BTC_NON_SEGWIT_ACCOUNT_PATH; + var preSegwitBackupFilename = isBsqWallet + ? WalletsSetup.PRE_SEGWIT_BSQ_WALLET_BACKUP + : WalletsSetup.PRE_SEGWIT_BTC_WALLET_BACKUP; + var walletFilename = isBsqWallet ? "bisq_BSQ.wallet" : "bisq_BTC.wallet"; + + if (nonSegwitAccountPath.equals(wallet.getActiveKeyChain().getAccountPath())) { if (wallet.isEncrypted() && aesKey == null) { // 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); + File backup = new File(directory, preSegwitBackupFilename); try { - FileUtil.copyFile(new File(directory, "bisq_BTC.wallet"), backup); + FileUtil.copyFile(new File(directory, walletFilename), backup); } catch (IOException e) { log.error(e.toString(), e); } - // Btc wallet does not have a native segwit keychain, we should add one. + // 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. @@ -568,7 +579,7 @@ public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey) { } DeterministicKeyChain nativeSegwitKeyChain = DeterministicKeyChain.builder().seed(seed) .outputScriptType(Script.ScriptType.P2WPKH) - .accountPath(new BisqKeyChainGroupStructure(false).accountPathFor(Script.ScriptType.P2WPKH)).build(); + .accountPath(new BisqKeyChainGroupStructure(isBsqWallet).accountPathFor(Script.ScriptType.P2WPKH)).build(); if (aesKey != null) { // If wallet is encrypted, encrypt the new keychain. KeyCrypter keyCrypter = wallet.getKeyCrypter(); @@ -576,7 +587,11 @@ public void maybeAddSegwitKeychain(Wallet wallet, KeyParameter aesKey) { } wallet.addAndActivateHDChain(nativeSegwitKeyChain); } - migratedWalletToSegwit.set(true); + if (isBsqWallet) { + migratedWalletToBsqSegwit.set(true); + } else { + migratedWalletToBtcSegwit.set(true); + } } public boolean stateStartingOrRunning() { 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 fad4ac30510..e6cba879a66 100644 --- a/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java +++ b/core/src/main/java/bisq/core/btc/setup/WalletsSetup.java @@ -109,7 +109,8 @@ @Slf4j public class WalletsSetup { - public static final String PRE_SEGWIT_WALLET_BACKUP = "pre_segwit_bisq_BTC.wallet.backup"; + public static final String PRE_SEGWIT_BTC_WALLET_BACKUP = "pre_segwit_bisq_BTC.wallet.backup"; + public static final String PRE_SEGWIT_BSQ_WALLET_BACKUP = "pre_segwit_bisq_BSQ.wallet.backup"; @Getter public final BooleanProperty walletsSetupFailed = new SimpleBooleanProperty(); @@ -427,12 +428,14 @@ public void clearBackups() { e.printStackTrace(); } - File segwitBackup = new File(walletDir, PRE_SEGWIT_WALLET_BACKUP); - try { - FileUtil.deleteFileIfExists(segwitBackup); - } catch (IOException e) { - log.error(e.toString(), e); - } + List.of(PRE_SEGWIT_BTC_WALLET_BACKUP, PRE_SEGWIT_BSQ_WALLET_BACKUP).forEach(filename -> { + File segwitBackup = new File(walletDir, filename); + try { + FileUtil.deleteFileIfExists(segwitBackup); + } catch (IOException e) { + log.error(e.toString(), e); + } + }); } 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 945ecd08ed1..c2f5d039da4 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -42,7 +42,6 @@ import org.bitcoinj.core.BlockChain; import org.bitcoinj.core.Coin; 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; @@ -50,6 +49,7 @@ import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; +import org.bitcoinj.script.Script; import org.bitcoinj.script.ScriptException; import org.bitcoinj.wallet.CoinSelection; import org.bitcoinj.wallet.CoinSelector; @@ -807,12 +807,13 @@ public Transaction getPreparedUnlockTx(TxOutput lockupTxOutput) throws AddressFo // Addresses /////////////////////////////////////////////////////////////////////////////////////////// - private LegacyAddress getChangeAddress() { + private Address getChangeAddress() { return getUnusedAddress(); } - public LegacyAddress getUnusedAddress() { - return (LegacyAddress) wallet.getIssuedReceiveAddresses().stream() + public Address getUnusedAddress() { + return wallet.getIssuedReceiveAddresses().stream() + .filter(address -> Script.ScriptType.P2WPKH.equals(address.getOutputScriptType())) .filter(this::isAddressUnused) .findAny() .orElse(wallet.freshReceiveAddress()); From 9760526aaa017c1ba2f3509bf71cb96ce61be9de Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 30 Jan 2021 02:35:53 +0000 Subject: [PATCH 4/4] Factor out shared segwit keychain setup to WalletsManager Move consecutive maybeAddSegwitKeychain(..) calls for the BTC & BSQ wallets from BisqSetup to WalletsManager, as the latter class is used for operations which should be applied to both wallets and this moves the logic closer to the wallet domain. Also migrate a BSQ address validator in one of the test mock classes from Base58AddressValidator to BitcoinAddressValidator (missed earlier). --- .../java/bisq/core/api/CoreWalletsService.java | 7 +------ core/src/main/java/bisq/core/app/BisqSetup.java | 5 +---- .../java/bisq/core/btc/wallet/WalletsManager.java | 14 ++++++++++++-- .../java/bisq/core/locale/MockTestnetCoin.java | 12 +++++------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 4ab6ff282a8..116e7125ecd 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -31,7 +31,6 @@ import bisq.core.btc.exceptions.WalletException; import bisq.core.btc.model.AddressEntry; import bisq.core.btc.model.BsqTransferModel; -import bisq.core.btc.setup.WalletsSetup; import bisq.core.btc.wallet.BsqTransferService; import bisq.core.btc.wallet.BsqWalletService; import bisq.core.btc.wallet.BtcWalletService; @@ -97,7 +96,6 @@ class CoreWalletsService { private final CoreContext coreContext; private final Balances balances; private final WalletsManager walletsManager; - private final WalletsSetup walletsSetup; private final BsqWalletService bsqWalletService; private final BsqTransferService bsqTransferService; private final BsqFormatter bsqFormatter; @@ -119,7 +117,6 @@ public CoreWalletsService(AppStartupState appStartupState, CoreContext coreContext, Balances balances, WalletsManager walletsManager, - WalletsSetup walletsSetup, BsqWalletService bsqWalletService, BsqTransferService bsqTransferService, BsqFormatter bsqFormatter, @@ -131,7 +128,6 @@ public CoreWalletsService(AppStartupState appStartupState, this.coreContext = coreContext; this.balances = balances; this.walletsManager = walletsManager; - this.walletsSetup = walletsSetup; this.bsqWalletService = bsqWalletService; this.bsqTransferService = bsqTransferService; this.bsqFormatter = bsqFormatter; @@ -581,8 +577,7 @@ private void maybeSetWalletsManagerKey() { if (btcWalletService.getAesKey() == null || bsqWalletService.getAesKey() == null) { KeyParameter aesKey = new KeyParameter(tempAesKey.getKey()); walletsManager.setAesKey(aesKey); - walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), aesKey, false); - walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().bsqWallet(), aesKey, true); + walletsManager.maybeAddSegwitKeychains(aesKey); } } diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index d83f2af6145..f39dbd9f69a 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -423,10 +423,7 @@ private void initWallet() { if (requestWalletPasswordHandler != null) { requestWalletPasswordHandler.accept(aesKey -> { walletsManager.setAesKey(aesKey); - walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().btcWallet(), - aesKey, false); - walletsSetup.getWalletConfig().maybeAddSegwitKeychain(walletsSetup.getWalletConfig().bsqWallet(), - aesKey, true); + walletsManager.maybeAddSegwitKeychains(aesKey); if (getResyncSpvSemaphore()) { if (showFirstPopupIfResyncSPVRequestedHandler != null) showFirstPopupIfResyncSPVRequestedHandler.run(); diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletsManager.java b/core/src/main/java/bisq/core/btc/wallet/WalletsManager.java index 2e6e9b6476a..df9705c67f6 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletsManager.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletsManager.java @@ -91,7 +91,9 @@ public String getWalletsAsString(boolean includePrivKeys) { return baseCurrencyWalletDetails + bsqWalletDetails; } - public void restoreSeedWords(@Nullable DeterministicSeed seed, ResultHandler resultHandler, ExceptionHandler exceptionHandler) { + public void restoreSeedWords(@Nullable DeterministicSeed seed, + ResultHandler resultHandler, + ExceptionHandler exceptionHandler) { walletsSetup.restoreSeedWords(seed, resultHandler, exceptionHandler); } @@ -140,7 +142,15 @@ public void setAesKey(KeyParameter aesKey) { tradeWalletService.setAesKey(aesKey); } - public DeterministicSeed getDecryptedSeed(KeyParameter aesKey, DeterministicSeed keyChainSeed, KeyCrypter keyCrypter) { + public void maybeAddSegwitKeychains(KeyParameter aesKey) { + var walletConfig = walletsSetup.getWalletConfig(); + walletConfig.maybeAddSegwitKeychain(walletConfig.btcWallet(), aesKey, false); + walletConfig.maybeAddSegwitKeychain(walletConfig.bsqWallet(), aesKey, true); + } + + public DeterministicSeed getDecryptedSeed(KeyParameter aesKey, + DeterministicSeed keyChainSeed, + KeyCrypter keyCrypter) { if (keyCrypter != null) { return keyChainSeed.decrypt(keyCrypter, "", aesKey); } else { diff --git a/core/src/test/java/bisq/core/locale/MockTestnetCoin.java b/core/src/test/java/bisq/core/locale/MockTestnetCoin.java index 8ed0e85e9e7..cc35e0c35c3 100644 --- a/core/src/test/java/bisq/core/locale/MockTestnetCoin.java +++ b/core/src/test/java/bisq/core/locale/MockTestnetCoin.java @@ -17,17 +17,15 @@ package bisq.core.locale; +import bisq.asset.AddressValidationResult; +import bisq.asset.BitcoinAddressValidator; +import bisq.asset.Coin; + import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.params.MainNetParams; import org.bitcoinj.params.RegTestParams; import org.bitcoinj.params.TestNet3Params; - - -import bisq.asset.AddressValidationResult; -import bisq.asset.Base58AddressValidator; -import bisq.asset.Coin; - public class MockTestnetCoin extends Coin { public MockTestnetCoin(Network network, NetworkParameters networkParameters) { @@ -55,7 +53,7 @@ public Regtest() { } } - public static class BSQAddressValidator extends Base58AddressValidator { + public static class BSQAddressValidator extends BitcoinAddressValidator { public BSQAddressValidator(NetworkParameters networkParameters) { super(networkParameters);