Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Segwit for the trade protocol #4612

Merged
merged 10 commits into from
Nov 5, 2020
Merged

Segwit for the trade protocol #4612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ef97daa
Use bitcoinj 0.15.8 (commit fcec3da)
oscarguindzberg Oct 15, 2020
161e220
BtcWalletService: Use segwit addresses
oscarguindzberg Sep 30, 2020
06e5091
TradeWalletService use P2WSH
oscarguindzberg Sep 22, 2020
4a05b6d
Revert "Construct dummy outputs with LegacyAddress"
oscarguindzberg Oct 6, 2020
22ba9a8
Explain why bitcoinSerialize(false) is used
oscarguindzberg Oct 6, 2020
3585dc9
Create the scriptCode the right way
oscarguindzberg Oct 6, 2020
953a5f0
Deal with P2WPKH has empty scriptSig
oscarguindzberg Oct 7, 2020
d1620c4
Revert "Validate AddressEntry.segwit"
oscarguindzberg Oct 6, 2020
a3d7c71
Set TRADE_PROTOCOL_VERSION to 3
oscarguindzberg Oct 15, 2020
dffa251
Remove unused imports
oscarguindzberg Oct 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ configure(subprojects) {

ext { // in alphabetical order
bcVersion = '1.63'
bitcoinjVersion = 'a733034'
bitcoinjVersion = 'fcec3da'
btcdCli4jVersion = '27b94333'
codecVersion = '1.13'
easybindVersion = '1.0.3'
Expand Down
7 changes: 5 additions & 2 deletions common/src/main/java/bisq/common/app/Version.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,13 @@ private static int getSubVersion(String version, int index) {

// The version no. of the current protocol. The offer holds that version.
// A taker will check the version of the offers to see if his version is compatible.
// Offers created with the old version will become invalid and have to be canceled.
// For the switch to version 2, offers created with the old version will become invalid and have to be canceled.
// For the switch to version 3, offers created with the old version can be migrated to version 3 just by opening
// the Bisq app.
// VERSION = 0.5.0 -> TRADE_PROTOCOL_VERSION = 1
// Version 1.2.2 -> TRADE_PROTOCOL_VERSION = 2
public static final int TRADE_PROTOCOL_VERSION = 2;
// Version 1.5.0 -> TRADE_PROTOCOL_VERSION = 3
public static final int TRADE_PROTOCOL_VERSION = 3;
private static int p2pMessageVersion;

public static final String BSQ_TX_VERSION = "1";
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/app/WalletAppSetup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void init(@Nullable Consumer<String> chainFileLockedExceptionHandler,
Runnable downloadCompleteHandler,
Runnable walletInitializedHandler) {
log.info("Initialize WalletAppSetup with BitcoinJ version {} and hash of BitcoinJ commit {}",
VersionMessage.BITCOINJ_VERSION, "a733034");
VersionMessage.BITCOINJ_VERSION, "fcec3da");

ObjectProperty<Throwable> walletServiceException = new SimpleObjectProperty<>();
btcInfoBinding = EasyBind.combine(walletsSetup.downloadPercentageProperty(),
Expand Down
4 changes: 0 additions & 4 deletions core/src/main/java/bisq/core/btc/model/AddressEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,6 @@ 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;
Expand Down
7 changes: 3 additions & 4 deletions core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,18 +579,17 @@ 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<AddressEntry> emptyAvailableAddressEntry = getAddressEntryListAsImmutableList().stream()
.filter(e -> AddressEntry.Context.AVAILABLE == e.getContext())
.filter(e -> isAddressUnused(e.getAddress()))
.filter(e -> Script.ScriptType.P2PKH.equals(e.getAddress().getOutputScriptType()))
.filter(e -> Script.ScriptType.P2WPKH.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);
DeterministicKey key = (DeterministicKey) wallet.findKeyFromAddress(wallet.freshReceiveAddress(Script.ScriptType.P2WPKH));
AddressEntry entry = new AddressEntry(key, context, offerId, true);
addressEntryList.addAddressEntry(entry);
return entry;
}
Expand Down
194 changes: 136 additions & 58 deletions core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 2 additions & 6 deletions core/src/main/java/bisq/core/btc/wallet/WalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
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;
Expand Down Expand Up @@ -325,11 +324,8 @@ public static void signTransactionInput(Wallet wallet,
}
} else if (ScriptPattern.isP2WPKH(scriptPubKey)) {
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();
// scriptCode is expected to have the format of a legacy P2PKH output script
Script scriptCode = ScriptBuilder.createP2PKHOutputScript(key);
Coin value = txIn.getValue();
TransactionSignature txSig = tx.calculateWitnessSignature(index, key, scriptCode, value,
Transaction.SigHash.ALL, false);
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSignsDelayedPayoutTx.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import bisq.common.taskrunner.TaskRunner;

import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.crypto.DeterministicKey;

Expand All @@ -46,7 +47,11 @@ protected void run() {
runInterceptHook();

Transaction preparedDelayedPayoutTx = checkNotNull(processModel.getPreparedDelayedPayoutTx());

BtcWalletService btcWalletService = processModel.getBtcWalletService();
NetworkParameters params = btcWalletService.getParams();
Transaction preparedDepositTx = new Transaction(params, processModel.getPreparedDepositTx());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not take the input from preparedDelayedPayoutTx? If that is not possible, I would prefer to pass over the deposit tx to the signDelayedPayoutTx method and do the deail work in the TradeWalletService to keep the transaction domain more isolated there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In bitcoin (the protocol) inputs do not include the amount.
In bitcoinj (the library) you can connect a TransactionInput to its related TransactionOutput and then ask the TransactionInput its value. But it is only possible if you connected both of them.
I moved the code to obtain the input value to TradeWalletService.

String id = processModel.getOffer().getId();

byte[] buyerMultiSigPubKey = processModel.getMyMultiSigPubKey();
Expand All @@ -58,6 +63,7 @@ protected void run() {
byte[] sellerMultiSigPubKey = processModel.getTradingPeer().getMultiSigPubKey();
byte[] delayedPayoutTxSignature = processModel.getTradeWalletService().signDelayedPayoutTx(
preparedDelayedPayoutTx,
preparedDepositTx,
myMultiSigKeyPair,
buyerMultiSigPubKey,
sellerMultiSigPubKey);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSignsDelayedPayoutTx.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import bisq.common.taskrunner.TaskRunner;

import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.crypto.DeterministicKey;

Expand All @@ -47,6 +48,9 @@ protected void run() {

Transaction preparedDelayedPayoutTx = checkNotNull(processModel.getPreparedDelayedPayoutTx());
BtcWalletService btcWalletService = processModel.getBtcWalletService();
NetworkParameters params = btcWalletService.getParams();
Transaction preparedDepositTx = new Transaction(params, processModel.getPreparedDepositTx());

String id = processModel.getOffer().getId();

byte[] sellerMultiSigPubKey = processModel.getMyMultiSigPubKey();
Expand All @@ -59,6 +63,7 @@ protected void run() {

byte[] delayedPayoutTxSignature = processModel.getTradeWalletService().signDelayedPayoutTx(
preparedDelayedPayoutTx,
preparedDepositTx,
myMultiSigKeyPair,
buyerMultiSigPubKey,
sellerMultiSigPubKey);
Expand Down
8 changes: 8 additions & 0 deletions desktop/src/main/java/bisq/desktop/main/overlays/windows/ManualPayoutTxWindow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@
import javax.inject.Inject;

import javafx.scene.Scene;
import javafx.scene.control.CheckBox;
import javafx.scene.input.KeyCode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;

import static bisq.desktop.util.FormBuilder.addCheckBox;
import static bisq.desktop.util.FormBuilder.addInputTextField;

// We don't translate here as it is for dev only purpose
Expand Down Expand Up @@ -116,6 +118,8 @@ private void addContent() {
InputTextField buyerPubKeyAsHex = addInputTextField(gridPane, ++rowIndex, "buyerPubKeyAsHex");
InputTextField sellerPubKeyAsHex = addInputTextField(gridPane, ++rowIndex, "sellerPubKeyAsHex");

CheckBox depositTxLegacy = addCheckBox(gridPane, ++rowIndex, "depositTxLegacy");

// Notes:
// Open with alt+g
// Priv key is only visible if pw protection is removed (wallet details data (alt+j))
Expand All @@ -136,6 +140,9 @@ private void addContent() {
sellerPubKeyAsHex.setText("");
sellerPrivateKeyAsHex.setText("");

depositTxLegacy.setAllowIndeterminate(false);
depositTxLegacy.setSelected(false);

actionButtonText("Sign and publish transaction");

TxBroadcaster.Callback callback = new TxBroadcaster.Callback() {
Expand Down Expand Up @@ -167,6 +174,7 @@ public void onFailure(TxBroadcastException exception) {
sellerPrivateKeyAsHex.getText(),
buyerPubKeyAsHex.getText(),
sellerPubKeyAsHex.getText(),
depositTxLegacy.isSelected(),
callback);
} catch (AddressFormatException | WalletException | TransactionVerificationException e) {
log.error(e.toString());
Expand Down
2 changes: 1 addition & 1 deletion gradle/witness/gradle-witness.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ dependencyVerification {
'com.github.bisq-network.netlayer:tor.external:a3606a592d6b6caa6a2fb7db224eaf43c6874c6730da4815bd37ad686b283dcb',
'com.github.bisq-network.netlayer:tor.native:b15aba7fe987185037791c7ec7c529cb001b90d723d047d54aab87aceb3b3d45',
'com.github.bisq-network.netlayer:tor:a974190aa3a031067ccd1dda28a3ae58cad14060792299d86ea38a05fb21afc5',
'com.github.bisq-network:bitcoinj:b8b6e4b8010f2b8d4aac7141c0809dea6d102c3ff3c06ceba78c2626d531b0af',
'com.github.bisq-network.bitcoinj:bitcoinj-core:8af7faa2155feff5afd1fa0fcea6fe7f7fa0d7ee977bdc648d1e73f3dcf2c754',
'com.github.cd2357.tor-binary:tor-binary-geoip:ae27b6aca1a3a50a046eb11e38202b6d21c2fcd2b8643bbeb5ea85e065fbc1be',
'com.github.cd2357.tor-binary:tor-binary-linux32:7b5d6770aa442ef6d235e8a9bfbaa7c62560690f9fe69ff03c7a752eae84f7dc',
'com.github.cd2357.tor-binary:tor-binary-linux64:24111fa35027599a750b0176392dc1e9417d919414396d1b221ac2e707eaba76',
Expand Down