From a5d8a87c388dfd76a6adcc779569e4caa7e7d522 Mon Sep 17 00:00:00 2001 From: Manfred Karrer Date: Thu, 19 Mar 2015 11:55:10 +0100 Subject: [PATCH] Fix bug with randomly failing tx signing caused by sorting of pub keys --- .../java/io/bitsquare/btc/AddressEntry.java | 4 +- .../main/java/io/bitsquare/btc/FeePolicy.java | 4 +- .../io/bitsquare/btc/TradeWalletService.java | 199 ++++++++++-------- .../java/io/bitsquare/btc/WalletService.java | 24 +-- .../common/taskrunner/SharedTaskModel.java | 3 + .../common/taskrunner/TaskRunner.java | 6 +- .../bitsquare/gui/main/debug/DebugView.java | 4 +- .../portfolio/pending/PendingTradesView.java | 2 +- .../io/bitsquare/persistence/Persistence.java | 10 + ...RequestOffererPublishDepositTxMessage.java | 12 +- .../trade/offerer/BuyerAsOffererProtocol.java | 7 +- .../offerer/models/BuyerAsOffererModel.java | 15 +- .../trade/offerer/models/TakerModel.java | 2 +- ...youtTx.java => CreateAndSignPayoutTx.java} | 6 +- .../tasks/CreateOffererDepositTxInputs.java | 2 +- ...RequestOffererPublishDepositTxMessage.java | 4 +- .../tasks/SignAndPublishDepositTx.java | 5 +- .../trade/taker/SellerAsTakerProtocol.java | 4 +- .../taker/models/SellerAsTakerModel.java | 7 + .../tasks/TakerCreatesAndSignsDepositTx.java | 2 +- core/src/main/resources/logback.xml | 2 +- 21 files changed, 180 insertions(+), 144 deletions(-) rename core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/{SignPayoutTx.java => CreateAndSignPayoutTx.java} (90%) diff --git a/core/src/main/java/io/bitsquare/btc/AddressEntry.java b/core/src/main/java/io/bitsquare/btc/AddressEntry.java index ba42c2a86f..694801aee1 100644 --- a/core/src/main/java/io/bitsquare/btc/AddressEntry.java +++ b/core/src/main/java/io/bitsquare/btc/AddressEntry.java @@ -49,8 +49,8 @@ public class AddressEntry implements Serializable { this.addressContext = addressContext; this.offerId = offerId; - pubKey = keyPair.getPubOnly().getPubKey(); - pubKeyHash = keyPair.getPubOnly().getPubKeyHash(); + pubKey = keyPair.getPubKey(); + pubKeyHash = keyPair.getPubKeyHash(); } public String getOfferId() { diff --git a/core/src/main/java/io/bitsquare/btc/FeePolicy.java b/core/src/main/java/io/bitsquare/btc/FeePolicy.java index bfd151bb36..959714c133 100644 --- a/core/src/main/java/io/bitsquare/btc/FeePolicy.java +++ b/core/src/main/java/io/bitsquare/btc/FeePolicy.java @@ -55,8 +55,8 @@ public class FeePolicy { takeOfferFeeAddress = "1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7"; break; case REGTEST: - createOfferFeeAddress = "mwjWBMW3tcvSDQWooybzumY8RFm4BkKSxZ"; - takeOfferFeeAddress = "mwjWBMW3tcvSDQWooybzumY8RFm4BkKSxZ"; + createOfferFeeAddress = "mxmKZruv9x9JLcEj6rZx6Hnm4LLAcQHtcr"; + takeOfferFeeAddress = "mxmKZruv9x9JLcEj6rZx6Hnm4LLAcQHtcr"; break; default: throw new BitsquareException("Unknown bitcoin network: %s", bitcoinNetwork); diff --git a/core/src/main/java/io/bitsquare/btc/TradeWalletService.java b/core/src/main/java/io/bitsquare/btc/TradeWalletService.java index 271adc7607..40cc39cccb 100644 --- a/core/src/main/java/io/bitsquare/btc/TradeWalletService.java +++ b/core/src/main/java/io/bitsquare/btc/TradeWalletService.java @@ -32,6 +32,7 @@ import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; +import org.bitcoinj.core.Utils; import org.bitcoinj.core.Wallet; import org.bitcoinj.crypto.TransactionSignature; import org.bitcoinj.kits.WalletAppKit; @@ -140,10 +141,10 @@ public class TradeWalletService { // Trade /////////////////////////////////////////////////////////////////////////////////////////// - public TransactionDataResult createOffererDepositTxInputs(Coin inputAmount, AddressEntry offererAddressEntry) throws + public Result createOffererDepositTxInputs(Coin offererInputAmount, AddressEntry offererAddressEntry) throws TransactionVerificationException, WalletException { log.trace("createOffererDepositTxInputs called"); - log.trace("inputAmount " + inputAmount.toFriendlyString()); + log.trace("offererInputAmount " + offererInputAmount.toFriendlyString()); log.trace("offererAddressEntry " + offererAddressEntry.toString()); // We pay the tx fee 2 times to the deposit tx: @@ -151,8 +152,8 @@ public class TradeWalletService { // 2. Will be added to the MS amount, so when publishing the payout tx the fee is already there and the outputs are not changed by fee reduction // The fee for the payout will be paid by the taker. - // inputAmount includes the tx fee. So we subtract the fee to get the dummyOutputAmount. - Coin dummyOutputAmount = inputAmount.subtract(FeePolicy.TX_FEE); + // offererInputAmount includes the tx fee. So we subtract the fee to get the dummyOutputAmount. + Coin dummyOutputAmount = offererInputAmount.subtract(FeePolicy.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. @@ -170,14 +171,13 @@ public class TradeWalletService { removeSignatures(dummyTX); verifyTransaction(dummyTX); - checkWalletConsistency(); // The created tx looks like: /* - IN[0] any input > inputAmount (including tx fee) (unsigned) + IN[0] any input > offererInputAmount (including tx fee) (unsigned) IN[1...n] optional inputs supported, but currently there is just 1 input (unsigned) - OUT[0] dummyOutputAmount (inputAmount - tx fee) - OUT[1] Optional Change = inputAmount - dummyOutputAmount - tx fee + OUT[0] dummyOutputAmount (offererInputAmount - tx fee) + OUT[1] Optional Change = offererInputAmount - dummyOutputAmount - tx fee OUT[2...n] optional more outputs are supported, but currently there is just max. 1 optional change output */ @@ -188,26 +188,23 @@ public class TradeWalletService { connectedOutputsForAllInputs.add(input.getConnectedOutput()); } - // Only save offerer outputs, the MS output is ignored + // Only save offerer outputs, the dummy output (index 1) is ignored List outputs = new ArrayList<>(); - for (TransactionOutput output : dummyTX.getOutputs()) { - if (output.equals(dummyOutput)) - continue; - outputs.add(output); + for (int i = 1; i < dummyTX.getOutputs().size(); i++) { + outputs.add(dummyTX.getOutputs().get(i)); } - return new TransactionDataResult(connectedOutputsForAllInputs, outputs); + return new Result(connectedOutputsForAllInputs, outputs); } - public TransactionDataResult takerCreatesAndSignsDepositTx(Coin takerInputAmount, - Coin msOutputAmount, - List offererConnectedOutputsForAllInputs, - List offererOutputs, - AddressEntry takerAddressInfo, - byte[] offererPubKey, - byte[] takerPubKey, - byte[] arbitratorPubKey) throws SigningException, - TransactionVerificationException, WalletException { + public Result takerCreatesAndSignsDepositTx(Coin takerInputAmount, + Coin msOutputAmount, + List offererConnectedOutputsForAllInputs, + List offererOutputs, + AddressEntry takerAddressInfo, + byte[] offererPubKey, + byte[] takerPubKey, + byte[] arbitratorPubKey) throws SigningException, TransactionVerificationException, WalletException { log.trace("takerCreatesAndSignsDepositTx called"); log.trace("takerInputAmount " + takerInputAmount.toFriendlyString()); log.trace("msOutputAmount " + msOutputAmount.toFriendlyString()); @@ -219,8 +216,10 @@ public class TradeWalletService { log.trace("arbitratorPubKey " + ECKey.fromPublicOnly(arbitratorPubKey).toString()); checkArgument(offererConnectedOutputsForAllInputs.size() > 0); + if (!Utils.HEX.encode(takerAddressInfo.getPubKey()).equals(Utils.HEX.encode(takerPubKey))) + throw new SigningException("TakerPubKey not matching key pair from addressEntry"); - // First we construct a dummy TX to get the inputs and outputs we want to use for the real deposit tx. Same as in first step at offerer. + // First we construct a dummy TX to get the inputs and outputs we want to use for the real deposit tx. Transaction dummyTx = new Transaction(params); Coin dummyOutputAmount = takerInputAmount.subtract(FeePolicy.TX_FEE); TransactionOutput dummyOutput = new TransactionOutput(params, dummyTx, dummyOutputAmount, new ECKey().toAddress(params)); @@ -233,7 +232,7 @@ public class TradeWalletService { takerOutputs.add(dummyTx.getOutput(i)); } - // Now we construct real deposit tx + // Now we construct the real deposit tx Transaction preparedDepositTx = new Transaction(params); // Add offerer inputs (normally its just 1 input) @@ -244,17 +243,17 @@ public class TradeWalletService { } // Add taker inputs - List connectedOutputsForAllTakerInputs = new ArrayList<>(); + List takerConnectedOutputsForAllInputs = new ArrayList<>(); for (TransactionInput input : takerInputs) { preparedDepositTx.addInput(input); - connectedOutputsForAllTakerInputs.add(input.getConnectedOutput()); + takerConnectedOutputsForAllInputs.add(input.getConnectedOutput()); } // Add MultiSig output - Script multiSigOutputScript = getP2SHMultiSigOutputScript(offererPubKey, takerPubKey, arbitratorPubKey); + Script p2SHMultiSigOutputScript = getP2SHMultiSigOutputScript(offererPubKey, takerPubKey, arbitratorPubKey); // Tx fee for deposit tx will be paid by offerer. - TransactionOutput msOutput = new TransactionOutput(params, preparedDepositTx, msOutputAmount, multiSigOutputScript.getProgram()); - preparedDepositTx.addOutput(msOutput); + TransactionOutput p2SHMultiSigOutput = new TransactionOutput(params, preparedDepositTx, msOutputAmount, p2SHMultiSigOutputScript.getProgram()); + preparedDepositTx.addOutput(p2SHMultiSigOutput); // Add optional offerer outputs for (TransactionOutput output : offererOutputs) { @@ -271,7 +270,7 @@ public class TradeWalletService { takersSpendingAmount = takersSpendingAmount.subtract(output.getValue()); } - // Sign inputs + // Sign inputs (start after offerer inputs) for (int i = offererConnectedOutputsForAllInputs.size(); i < preparedDepositTx.getInputs().size(); i++) { TransactionInput input = preparedDepositTx.getInput(i); signInput(preparedDepositTx, input, i); @@ -285,23 +284,23 @@ public class TradeWalletService { throw new TransactionVerificationException("Takers input amount does not match required value."); verifyTransaction(preparedDepositTx); - checkWalletConsistency(); printTxWithInputs("preparedDepositTx", preparedDepositTx); - return new TransactionDataResult(preparedDepositTx, connectedOutputsForAllTakerInputs, takerOutputs); + return new Result(preparedDepositTx, takerConnectedOutputsForAllInputs, takerOutputs); } - public void offererSignsAndPublishTx(Transaction takersDepositTx, - List offererConnectedOutputsForAllInputs, - List takerConnectedOutputsForAllInputs, - List offererOutputs, - Coin offererInputAmount, - byte[] offererPubKey, - byte[] takerPubKey, - byte[] arbitratorPubKey, - FutureCallback callback) throws SigningException, TransactionVerificationException, WalletException { + public void offererSignsAndPublishDepositTx(Transaction takersPreparedDepositTx, + List offererConnectedOutputsForAllInputs, + List takerConnectedOutputsForAllInputs, + List offererOutputs, + Coin offererInputAmount, + byte[] offererPubKey, + byte[] takerPubKey, + byte[] arbitratorPubKey, + FutureCallback callback) throws SigningException, TransactionVerificationException, + WalletException { log.trace("offererSignsAndPublishTx called"); - log.trace("takersDepositTx " + takersDepositTx.toString()); + log.trace("takersPreparedDepositTx " + takersPreparedDepositTx.toString()); log.trace("offererConnectedOutputsForAllInputs " + offererConnectedOutputsForAllInputs.toString()); log.trace("takerConnectedOutputsForAllInputs " + takerConnectedOutputsForAllInputs.toString()); log.trace("offererOutputs " + offererOutputs.toString()); @@ -314,11 +313,12 @@ public class TradeWalletService { checkArgument(takerConnectedOutputsForAllInputs.size() > 0); // Check if takers Multisig script is identical to mine - Script multiSigOutputScript = getP2SHMultiSigOutputScript(offererPubKey, takerPubKey, arbitratorPubKey); - if (!takersDepositTx.getOutput(0).getScriptPubKey().equals(multiSigOutputScript)) - throw new TransactionVerificationException("Takers multiSigOutputScript does not match to my multiSigOutputScript"); + Script p2SHMultiSigOutputScript = getP2SHMultiSigOutputScript(offererPubKey, takerPubKey, arbitratorPubKey); + if (!takersPreparedDepositTx.getOutput(0).getScriptPubKey().equals(p2SHMultiSigOutputScript)) + throw new TransactionVerificationException("Takers p2SHMultiSigOutputScript does not match to my p2SHMultiSigOutputScript"); - // The outpoints are not available from the serialized takersDepositTx, so we cannot use that tx directly, but we use it to construct a new depositTx + // The outpoints are not available from the serialized takersPreparedDepositTx, so we cannot use that tx directly, but we use it to construct a new + // depositTx Transaction depositTx = new Transaction(params); // Add offerer inputs @@ -336,8 +336,8 @@ public class TradeWalletService { for (TransactionOutput connectedOutputForInput : takerConnectedOutputsForAllInputs) { TransactionOutPoint outPoint = new TransactionOutPoint(params, connectedOutputForInput.getIndex(), connectedOutputForInput.getParentTransaction()); - // We grab the signature from the takersDepositTx and apply it to the new tx input - TransactionInput takerInput = takersDepositTx.getInputs().get(offererConnectedOutputsForAllInputs.size()); + // We grab the signature from the takersPreparedDepositTx and apply it to the new tx input + TransactionInput takerInput = takersPreparedDepositTx.getInputs().get(offererConnectedOutputsForAllInputs.size()); byte[] scriptProgram = takerInput.getScriptSig().getProgram(); if (scriptProgram.length == 0) throw new TransactionVerificationException("Inputs from taker not singed."); @@ -346,8 +346,8 @@ public class TradeWalletService { depositTx.addInput(transactionInput); } - // Add all outputs from takersDepositTx to depositTx - for (TransactionOutput output : takersDepositTx.getOutputs()) { + // Add all outputs from takersPreparedDepositTx to depositTx + for (TransactionOutput output : takersPreparedDepositTx.getOutputs()) { depositTx.addOutput(output); } @@ -383,6 +383,7 @@ public class TradeWalletService { depositTx = new Transaction(params, depositTx.bitcoinSerialize()); try { + // TODO check if that is correct wallet.receivePending(depositTx, null, true); } catch (Throwable t) { log.error(t.getMessage()); @@ -399,18 +400,24 @@ public class TradeWalletService { byte[] offererPubKey, byte[] takerPubKey, byte[] arbitratorPubKey) - throws AddressFormatException, TransactionVerificationException { + throws AddressFormatException, TransactionVerificationException, SigningException { log.trace("offererCreatesAndSignsPayoutTx called"); log.trace("depositTx " + depositTx.toString()); log.trace("offererPayoutAmount " + offererPayoutAmount.toFriendlyString()); log.trace("takerPayoutAmount " + takerPayoutAmount.toFriendlyString()); - log.trace("takerPayoutAddressString " + takerPayoutAddressString); log.trace("offererAddressEntry " + offererAddressEntry.toString()); + log.trace("takerPayoutAddressString " + takerPayoutAddressString); log.trace("offererPubKey " + ECKey.fromPublicOnly(offererPubKey).toString()); log.trace("takerPubKey " + ECKey.fromPublicOnly(takerPubKey).toString()); log.trace("arbitratorPubKey " + ECKey.fromPublicOnly(arbitratorPubKey).toString()); - Transaction preparedPayoutTx = createPayoutTx(depositTx, offererPayoutAmount, takerPayoutAmount, offererAddressEntry.getAddressString(), + if (!Utils.HEX.encode(offererAddressEntry.getPubKey()).equals(Utils.HEX.encode(offererPubKey))) + throw new SigningException("OffererPubKey not matching key pair from addressEntry"); + + Transaction preparedPayoutTx = createPayoutTx(depositTx, + offererPayoutAmount, + takerPayoutAmount, + offererAddressEntry.getAddressString(), takerPayoutAddressString); // MS redeemScript Script redeemScript = getMultiSigRedeemScript(offererPubKey, takerPubKey, arbitratorPubKey); @@ -422,6 +429,9 @@ public class TradeWalletService { printTxWithInputs("preparedPayoutTx", preparedPayoutTx); log.trace("offererSignature r " + offererSignature.toCanonicalised().r.toString()); log.trace("offererSignature s " + offererSignature.toCanonicalised().s.toString()); + Sha256Hash hashForSignature = preparedPayoutTx.hashForSignature(0, redeemScript.getProgram(), (byte) 1); + log.trace("hashForSignature " + Utils.HEX.encode(hashForSignature.getBytes())); + return offererSignature.encodeToDER(); } @@ -435,11 +445,11 @@ public class TradeWalletService { byte[] takerPubKey, byte[] arbitratorPubKey, FutureCallback callback) - throws AddressFormatException, TransactionVerificationException, WalletException { + throws AddressFormatException, TransactionVerificationException, WalletException, SigningException { log.trace("takerSignsAndPublishPayoutTx called"); log.trace("depositTx " + depositTx.toString()); - log.trace("offererSignature r " + ECKey.ECDSASignature.decodeFromDER(offererSignature).toCanonicalised().r.toString()); - log.trace("offererSignature s " + ECKey.ECDSASignature.decodeFromDER(offererSignature).toCanonicalised().s.toString()); + log.trace("offererSignature r " + ECKey.ECDSASignature.decodeFromDER(offererSignature).r.toString()); + log.trace("offererSignature s " + ECKey.ECDSASignature.decodeFromDER(offererSignature).s.toString()); log.trace("offererPayoutAmount " + offererPayoutAmount.toFriendlyString()); log.trace("takerPayoutAmount " + takerPayoutAmount.toFriendlyString()); log.trace("offererAddressString " + offererAddressString); @@ -448,20 +458,36 @@ public class TradeWalletService { log.trace("takerPubKey " + ECKey.fromPublicOnly(takerPubKey).toString()); log.trace("arbitratorPubKey " + ECKey.fromPublicOnly(arbitratorPubKey).toString()); - Transaction payoutTx = createPayoutTx(depositTx, offererPayoutAmount, takerPayoutAmount, offererAddressString, takerAddressEntry.getAddressString()); - // MS redeemScript + if (!Utils.HEX.encode(takerAddressEntry.getPubKey()).equals(Utils.HEX.encode(takerPubKey))) + throw new SigningException("TakerPubKey not matching key pair from addressEntry"); + + Transaction payoutTx = createPayoutTx(depositTx, + offererPayoutAmount, + takerPayoutAmount, + offererAddressString, + takerAddressEntry.getAddressString()); Script redeemScript = getMultiSigRedeemScript(offererPubKey, takerPubKey, arbitratorPubKey); Sha256Hash sigHash = payoutTx.hashForSignature(0, redeemScript, Transaction.SigHash.ALL, false); ECKey.ECDSASignature takerSignature = takerAddressEntry.getKeyPair().sign(sigHash).toCanonicalised(); - TransactionSignature offererTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(offererSignature).toCanonicalised(), + + log.trace("takerSignature r " + takerSignature.r.toString()); + log.trace("takerSignature s " + takerSignature.s.toString()); + + Sha256Hash hashForSignature = payoutTx.hashForSignature(0, redeemScript.getProgram(), (byte) 1); + log.trace("hashForSignature " + Utils.HEX.encode(hashForSignature.getBytes())); + + TransactionSignature offererTxSig = new TransactionSignature(ECKey.ECDSASignature.decodeFromDER(offererSignature), Transaction.SigHash.ALL, false); TransactionSignature takerTxSig = new TransactionSignature(takerSignature, Transaction.SigHash.ALL, false); - Script inputScript = ScriptBuilder.createP2SHMultiSigInputScript(ImmutableList.of(offererTxSig, takerTxSig), redeemScript); + // Take care of order of signatures. See comment below at getMultiSigRedeemScript + Script inputScript = ScriptBuilder.createP2SHMultiSigInputScript(ImmutableList.of(takerTxSig, offererTxSig), redeemScript); TransactionInput input = payoutTx.getInput(0); input.setScriptSig(inputScript); verifyTransaction(payoutTx); checkWalletConsistency(); + checkScriptSig(payoutTx, input, 0); + input.verify(input.getConnectedOutput()); printTxWithInputs("payoutTx", payoutTx); @@ -473,31 +499,40 @@ public class TradeWalletService { // Private methods /////////////////////////////////////////////////////////////////////////////////////////// - private Script getP2SHMultiSigOutputScript(byte[] offererPubKey, byte[] takerPubKey, byte[] arbitratorPubKey) { - ECKey offererKey = ECKey.fromPublicOnly(offererPubKey); - ECKey takerKey = ECKey.fromPublicOnly(takerPubKey); - ECKey arbitratorKey = ECKey.fromPublicOnly(arbitratorPubKey); - List keys = ImmutableList.of(offererKey, takerKey, arbitratorKey); - return ScriptBuilder.createP2SHOutputScript(2, keys); - } - + // Don't use ScriptBuilder.createRedeemScript and ScriptBuilder.createP2SHOutputScript as they use a sorting + // (Collections.sort(pubKeys, ECKey.PUBKEY_COMPARATOR);) which can lead to a non-matching list of signatures with pubKeys and the executeMultiSig does + // not iterate all possible combinations of sig/pubKeys leading to a verification fault. That nasty bug happens just randomly as the list after sorting + // might differ from the provided one or not. + // Changing the while loop in executeMultiSig to fix that does not help as the reference implementation seems to behave the same (not iterating all + // possibilities) . + // Furthermore the executed list is reversed to the provided. + // Best practice is to provide the list sorted by the least probable successful candidates first (arbitrator is first -> will be last in execution loop, so + // avoiding unneeded expensive ECKey.verify calls) private Script getMultiSigRedeemScript(byte[] offererPubKey, byte[] takerPubKey, byte[] arbitratorPubKey) { ECKey offererKey = ECKey.fromPublicOnly(offererPubKey); ECKey takerKey = ECKey.fromPublicOnly(takerPubKey); ECKey arbitratorKey = ECKey.fromPublicOnly(arbitratorPubKey); - List keys = ImmutableList.of(offererKey, takerKey, arbitratorKey); - return ScriptBuilder.createRedeemScript(2, keys); + // Take care of sorting! + List keys = ImmutableList.of(arbitratorKey, takerKey, offererKey); + return ScriptBuilder.createMultiSigOutputScript(2, keys); } - private Transaction createPayoutTx(Transaction depositTx, Coin offererPayoutAmount, Coin takerPayoutAmount, - String offererAddressString, String takerAddressString) throws AddressFormatException { + private Script getP2SHMultiSigOutputScript(byte[] offererPubKey, byte[] takerPubKey, byte[] arbitratorPubKey) { + return ScriptBuilder.createP2SHOutputScript(getMultiSigRedeemScript(offererPubKey, takerPubKey, arbitratorPubKey)); + } - TransactionOutput multiSigOutput = depositTx.getOutput(0); - Transaction tx = new Transaction(params); - tx.addInput(multiSigOutput); - tx.addOutput(offererPayoutAmount, new Address(params, offererAddressString)); - tx.addOutput(takerPayoutAmount, new Address(params, takerAddressString)); - return tx; + private Transaction createPayoutTx(Transaction depositTx, + Coin offererPayoutAmount, + Coin takerPayoutAmount, + String offererAddressString, + String takerAddressString) throws AddressFormatException { + + TransactionOutput p2SHMultiSigOutput = depositTx.getOutput(0); + Transaction transaction = new Transaction(params); + transaction.addInput(p2SHMultiSigOutput); + transaction.addOutput(offererPayoutAmount, new Address(params, offererAddressString)); + transaction.addOutput(takerPayoutAmount, new Address(params, takerAddressString)); + return transaction; } private static void printTxWithInputs(String tracePrefix, Transaction tx) { @@ -588,7 +623,7 @@ public class TradeWalletService { // Inner classes /////////////////////////////////////////////////////////////////////////////////////////// - public class TransactionDataResult { + public class Result { private List connectedOutputsForAllInputs; private List outputs; private Transaction depositTx; @@ -596,12 +631,12 @@ public class TradeWalletService { private byte[] offererSignature; - public TransactionDataResult(List connectedOutputsForAllInputs, List outputs) { + public Result(List connectedOutputsForAllInputs, List outputs) { this.connectedOutputsForAllInputs = connectedOutputsForAllInputs; this.outputs = outputs; } - public TransactionDataResult(Transaction depositTx, List connectedOutputsForAllInputs, List outputs) { + public Result(Transaction depositTx, List connectedOutputsForAllInputs, List outputs) { this.depositTx = depositTx; this.connectedOutputsForAllInputs = connectedOutputsForAllInputs; this.outputs = outputs; diff --git a/core/src/main/java/io/bitsquare/btc/WalletService.java b/core/src/main/java/io/bitsquare/btc/WalletService.java index 9f369270b3..edeb390454 100644 --- a/core/src/main/java/io/bitsquare/btc/WalletService.java +++ b/core/src/main/java/io/bitsquare/btc/WalletService.java @@ -61,11 +61,8 @@ import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; -import javax.annotation.concurrent.GuardedBy; - import javax.inject.Inject; import javax.inject.Named; @@ -82,7 +79,6 @@ import static org.bitcoinj.script.ScriptOpCodes.OP_RETURN; public class WalletService { private static final Logger log = LoggerFactory.getLogger(WalletService.class); - private static final String LOCK_NAME = "lock"; public static final String DIR_KEY = "wallet.dir"; public static final String PREFIX_KEY = "wallet.prefix"; @@ -90,7 +86,6 @@ public class WalletService { private final List addressConfidenceListeners = new CopyOnWriteArrayList<>(); private final List txConfidenceListeners = new CopyOnWriteArrayList<>(); private final List balanceListeners = new CopyOnWriteArrayList<>(); - private final ReentrantLock lock = Threading.lock(LOCK_NAME); private final ObservableDownloadListener downloadListener = new ObservableDownloadListener(); private final Observable downloadProgress = downloadListener.getObservable(); @@ -108,7 +103,7 @@ public class WalletService { private Wallet wallet; private AddressEntry registrationAddressEntry; private AddressEntry arbitratorDepositAddressEntry; - private @GuardedBy(LOCK_NAME) List addressEntryList = new ArrayList<>(); + private List addressEntryList = new ArrayList<>(); private TradeWalletService tradeWalletService; @@ -221,12 +216,10 @@ public class WalletService { } else { // First time - lock.lock(); DeterministicKey registrationKey = wallet.currentReceiveKey(); registrationAddressEntry = new AddressEntry(registrationKey, params, AddressEntry.AddressContext.REGISTRATION_FEE); addressEntryList.add(registrationAddressEntry); - lock.unlock(); saveAddressInfoList(); } } @@ -300,9 +293,8 @@ public class WalletService { public AddressEntry getAddressEntry(String offerId) { log.trace("getAddressEntry called with offerId " + offerId); - Optional addressEntry = getAddressEntryList().stream().filter(e -> - offerId.equals(e.getOfferId())).findFirst(); - + Optional addressEntry = getAddressEntryList().stream().filter(e -> offerId.equals(e.getOfferId())).findFirst(); + if (addressEntry.isPresent()) return addressEntry.get(); else @@ -315,12 +307,11 @@ public class WalletService { /////////////////////////////////////////////////////////////////////////////////////////// private AddressEntry getNewAddressEntry(AddressEntry.AddressContext addressContext, String offerId) { - lock.lock(); + log.trace("getNewAddressEntry called with offerId " + offerId); DeterministicKey key = wallet.freshReceiveKey(); AddressEntry addressEntry = new AddressEntry(key, params, addressContext, offerId); addressEntryList.add(addressEntry); saveAddressInfoList(); - lock.unlock(); return addressEntry; } @@ -552,12 +543,7 @@ public class WalletService { /////////////////////////////////////////////////////////////////////////////////////////// private void saveAddressInfoList() { - lock.lock(); - try { - persistence.write(this, "addressEntryList", addressEntryList); - } finally { - lock.unlock(); - } + persistence.write(this, "addressEntryList", addressEntryList); } private static void printTxWithInputs(String tracePrefix, Transaction tx) { diff --git a/core/src/main/java/io/bitsquare/common/taskrunner/SharedTaskModel.java b/core/src/main/java/io/bitsquare/common/taskrunner/SharedTaskModel.java index 3c1f10098c..d6a07aaeb7 100644 --- a/core/src/main/java/io/bitsquare/common/taskrunner/SharedTaskModel.java +++ b/core/src/main/java/io/bitsquare/common/taskrunner/SharedTaskModel.java @@ -25,4 +25,7 @@ public class SharedTaskModel { public void persist() { } + + public void onComplete() { + } } diff --git a/core/src/main/java/io/bitsquare/common/taskrunner/TaskRunner.java b/core/src/main/java/io/bitsquare/common/taskrunner/TaskRunner.java index f5f80fe502..f291af588e 100644 --- a/core/src/main/java/io/bitsquare/common/taskrunner/TaskRunner.java +++ b/core/src/main/java/io/bitsquare/common/taskrunner/TaskRunner.java @@ -77,14 +77,10 @@ public class TaskRunner { void handleComplete() { log.trace("Task completed: " + currentTask.getSimpleName()); - persistModel(); + sharedModel.persist(); next(); } - protected void persistModel() { - // sharedModel.persist(); - } - void handleErrorMessage(String errorMessage) { log.error("Task failed: " + currentTask.getSimpleName()); log.error("errorMessage: " + errorMessage); diff --git a/core/src/main/java/io/bitsquare/gui/main/debug/DebugView.java b/core/src/main/java/io/bitsquare/gui/main/debug/DebugView.java index 150fe4cd23..16f2461f6c 100644 --- a/core/src/main/java/io/bitsquare/gui/main/debug/DebugView.java +++ b/core/src/main/java/io/bitsquare/gui/main/debug/DebugView.java @@ -38,7 +38,7 @@ import io.bitsquare.trade.protocol.trade.offerer.tasks.SendBankTransferStartedMe import io.bitsquare.trade.protocol.trade.offerer.tasks.SendDepositTxIdToTaker; import io.bitsquare.trade.protocol.trade.offerer.tasks.SetupListenerForBlockChainConfirmation; import io.bitsquare.trade.protocol.trade.offerer.tasks.SignAndPublishDepositTx; -import io.bitsquare.trade.protocol.trade.offerer.tasks.SignPayoutTx; +import io.bitsquare.trade.protocol.trade.offerer.tasks.CreateAndSignPayoutTx; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyAndSignContract; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyTakeOfferFeePayment; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyTakerAccount; @@ -113,7 +113,7 @@ public class DebugView extends InitializableView { SetupListenerForBlockChainConfirmation.class, SendDepositTxIdToTaker.class, - SignPayoutTx.class, + CreateAndSignPayoutTx.class, VerifyTakeOfferFeePayment.class, SendBankTransferStartedMessage.class, diff --git a/core/src/main/java/io/bitsquare/gui/main/portfolio/pending/PendingTradesView.java b/core/src/main/java/io/bitsquare/gui/main/portfolio/pending/PendingTradesView.java index 364abd402f..0354103311 100644 --- a/core/src/main/java/io/bitsquare/gui/main/portfolio/pending/PendingTradesView.java +++ b/core/src/main/java/io/bitsquare/gui/main/portfolio/pending/PendingTradesView.java @@ -107,7 +107,7 @@ public class PendingTradesView extends ActivatableViewAndModel takerConnectedOutputsForAllInputs; public final List takerOutputs; @@ -47,8 +47,8 @@ public class RequestOffererPublishDepositTxMessage extends TradeMessage implemen PublicKey takerMessagePublicKey, String takerContractAsJson, String takerContractSignature, - String takerPayoutAddress, - Transaction takersDepositTx, + String takerPayoutAddressString, + Transaction takersPreparedDepositTx, List takerConnectedOutputsForAllInputs, List takerOutputs) { this.tradeId = tradeId; @@ -57,8 +57,8 @@ public class RequestOffererPublishDepositTxMessage extends TradeMessage implemen this.takerMessagePublicKey = takerMessagePublicKey; this.takerContractAsJson = takerContractAsJson; this.takerContractSignature = takerContractSignature; - this.takerPayoutAddress = takerPayoutAddress; - this.takersDepositTx = takersDepositTx; + this.takerPayoutAddressString = takerPayoutAddressString; + this.takersPreparedDepositTx = takersPreparedDepositTx; this.takerConnectedOutputsForAllInputs = takerConnectedOutputsForAllInputs; this.takerOutputs = takerOutputs; } diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/BuyerAsOffererProtocol.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/BuyerAsOffererProtocol.java index 1a24f42ccd..2578fa1467 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/BuyerAsOffererProtocol.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/BuyerAsOffererProtocol.java @@ -35,7 +35,7 @@ import io.bitsquare.trade.protocol.trade.offerer.tasks.SendBankTransferStartedMe import io.bitsquare.trade.protocol.trade.offerer.tasks.SendDepositTxIdToTaker; import io.bitsquare.trade.protocol.trade.offerer.tasks.SetupListenerForBlockChainConfirmation; import io.bitsquare.trade.protocol.trade.offerer.tasks.SignAndPublishDepositTx; -import io.bitsquare.trade.protocol.trade.offerer.tasks.SignPayoutTx; +import io.bitsquare.trade.protocol.trade.offerer.tasks.CreateAndSignPayoutTx; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyAndSignContract; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyTakeOfferFeePayment; import io.bitsquare.trade.protocol.trade.offerer.tasks.VerifyTakerAccount; @@ -162,7 +162,7 @@ public class BuyerAsOffererProtocol { } ); taskRunner.addTasks( - SignPayoutTx.class, + CreateAndSignPayoutTx.class, VerifyTakeOfferFeePayment.class, SendBankTransferStartedMessage.class ); @@ -180,6 +180,9 @@ public class BuyerAsOffererProtocol { BuyerAsOffererTaskRunner taskRunner = new BuyerAsOffererTaskRunner<>(model, () -> { log.debug("sequence at handlePayoutTxPublishedMessage completed"); + + // we are done! + model.onComplete(); }, (errorMessage) -> { log.error(errorMessage); diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/BuyerAsOffererModel.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/BuyerAsOffererModel.java index 487abf0b14..6c451b0f68 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/BuyerAsOffererModel.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/BuyerAsOffererModel.java @@ -26,8 +26,6 @@ import io.bitsquare.trade.TradeMessageService; import io.bitsquare.trade.protocol.trade.SharedTradeModel; import io.bitsquare.user.User; -import org.bitcoinj.core.Transaction; - import java.io.Serializable; import org.slf4j.Logger; @@ -42,7 +40,6 @@ public class BuyerAsOffererModel extends SharedTradeModel implements Serializabl public final OffererModel offerer; // written by tasks - private Transaction publishedDepositTx; private String takeOfferFeeTxId; public BuyerAsOffererModel(Trade trade, @@ -66,7 +63,6 @@ public class BuyerAsOffererModel extends SharedTradeModel implements Serializabl BuyerAsOffererModel persistedModel = (BuyerAsOffererModel) serializable; log.debug("Model reconstructed form persisted model."); - setPublishedDepositTx(persistedModel.getPublishedDepositTx()); setTakeOfferFeeTxId(persistedModel.takeOfferFeeTxId); taker = persistedModel.taker; @@ -84,6 +80,7 @@ public class BuyerAsOffererModel extends SharedTradeModel implements Serializabl offerer.accountId = user.getAccountId(); offerer.messagePubKey = user.getMessagePubKey(); offerer.pubKey = offerer.addressEntry.getPubKey(); + log.debug("BuyerAsOffererModel addressEntry " + offerer.addressEntry); } // Get called form taskRunner after each completed task @@ -92,12 +89,10 @@ public class BuyerAsOffererModel extends SharedTradeModel implements Serializabl persistence.write(this, "BuyerAsOffererModel_" + id, this); } - public Transaction getPublishedDepositTx() { - return publishedDepositTx; - } - - public void setPublishedDepositTx(Transaction publishedDepositTx) { - this.publishedDepositTx = publishedDepositTx; + @Override + public void onComplete() { + // Just in case of successful completion we delete our persisted object + persistence.remove(this, "BuyerAsOffererModel_" + id); } public String getTakeOfferFeeTxId() { diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/TakerModel.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/TakerModel.java index 296869dfff..daf572c9e7 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/TakerModel.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/models/TakerModel.java @@ -41,7 +41,7 @@ public class TakerModel implements Serializable { public String contractAsJson;//TODO only write access now, missing impl. public String contractSignature; public Coin payoutAmount; - public Transaction depositTx; + public Transaction preparedDepositTx; public List connectedOutputsForAllInputs; public String payoutAddressString; public byte[] pubKey; diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignPayoutTx.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateAndSignPayoutTx.java similarity index 90% rename from core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignPayoutTx.java rename to core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateAndSignPayoutTx.java index eb27eaa9fe..383b2b8818 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignPayoutTx.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateAndSignPayoutTx.java @@ -27,10 +27,10 @@ import org.bitcoinj.core.Coin; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class SignPayoutTx extends Task { - private static final Logger log = LoggerFactory.getLogger(SignPayoutTx.class); +public class CreateAndSignPayoutTx extends Task { + private static final Logger log = LoggerFactory.getLogger(CreateAndSignPayoutTx.class); - public SignPayoutTx(TaskRunner taskHandler, BuyerAsOffererModel model) { + public CreateAndSignPayoutTx(TaskRunner taskHandler, BuyerAsOffererModel model) { super(taskHandler, model); } diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateOffererDepositTxInputs.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateOffererDepositTxInputs.java index 9cea37fddf..24c3b83fab 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateOffererDepositTxInputs.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateOffererDepositTxInputs.java @@ -39,7 +39,7 @@ public class CreateOffererDepositTxInputs extends Task { protected void doRun() { try { Coin offererInputAmount = model.trade.getSecurityDeposit().add(FeePolicy.TX_FEE); - TradeWalletService.TransactionDataResult result = model.tradeWalletService.createOffererDepositTxInputs(offererInputAmount, + TradeWalletService.Result result = model.tradeWalletService.createOffererDepositTxInputs(offererInputAmount, model.offerer.addressEntry); model.offerer.connectedOutputsForAllInputs = result.getConnectedOutputsForAllInputs(); diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/ProcessRequestOffererPublishDepositTxMessage.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/ProcessRequestOffererPublishDepositTxMessage.java index 767e6c4a1d..7b0edc972b 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/ProcessRequestOffererPublishDepositTxMessage.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/ProcessRequestOffererPublishDepositTxMessage.java @@ -46,8 +46,8 @@ public class ProcessRequestOffererPublishDepositTxMessage extends Task 0); diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignAndPublishDepositTx.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignAndPublishDepositTx.java index 00da382582..46f6c6e2f4 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignAndPublishDepositTx.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignAndPublishDepositTx.java @@ -44,8 +44,8 @@ public class SignAndPublishDepositTx extends Task { protected void doRun() { try { Coin offererInputAmount = model.trade.getSecurityDeposit().add(FeePolicy.TX_FEE); - model.tradeWalletService.offererSignsAndPublishTx( - model.taker.depositTx, + model.tradeWalletService.offererSignsAndPublishDepositTx( + model.taker.preparedDepositTx, model.offerer.connectedOutputsForAllInputs, model.taker.connectedOutputsForAllInputs, model.offerer.outputs, @@ -58,7 +58,6 @@ public class SignAndPublishDepositTx extends Task { public void onSuccess(Transaction transaction) { log.trace("offererSignAndPublishTx succeeded " + transaction); - model.setPublishedDepositTx(transaction); model.trade.setDepositTx(transaction); model.trade.setState(Trade.State.DEPOSIT_PUBLISHED); diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/SellerAsTakerProtocol.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/SellerAsTakerProtocol.java index 067ecdf7e6..c8e88fe248 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/SellerAsTakerProtocol.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/SellerAsTakerProtocol.java @@ -166,6 +166,9 @@ public class SellerAsTakerProtocol { SellerAsTakerTaskRunner taskRunner = new SellerAsTakerTaskRunner<>(model, () -> { log.debug("taskRunner at handleFiatReceivedUIEvent completed"); + + // we are done! + model.onComplete(); }, (errorMessage) -> { log.error(errorMessage); @@ -179,7 +182,6 @@ public class SellerAsTakerProtocol { taskRunner.run(); } - /////////////////////////////////////////////////////////////////////////////////////////// // Massage dispatcher /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/models/SellerAsTakerModel.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/models/SellerAsTakerModel.java index fa7b2587d1..060d23e4e1 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/models/SellerAsTakerModel.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/models/SellerAsTakerModel.java @@ -96,6 +96,13 @@ public class SellerAsTakerModel extends SharedTradeModel implements Serializable persistence.write(this, "SellerAsTakerModel_" + id, this); } + @Override + public void onComplete() { + // Just in case of successful completion we delete our persisted object + persistence.remove(this, "SellerAsTakerModel_" + id); + } + + public Transaction getTakeOfferFeeTx() { return takeOfferFeeTx; } diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/tasks/TakerCreatesAndSignsDepositTx.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/tasks/TakerCreatesAndSignsDepositTx.java index 8c24ae0368..facf6de112 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/tasks/TakerCreatesAndSignsDepositTx.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/taker/tasks/TakerCreatesAndSignsDepositTx.java @@ -42,7 +42,7 @@ public class TakerCreatesAndSignsDepositTx extends Task { Coin takerInputAmount = model.trade.getTradeAmount().add(model.trade.getSecurityDeposit()).add(FeePolicy.TX_FEE); Coin msOutputAmount = takerInputAmount.add(model.trade.getSecurityDeposit()); - TradeWalletService.TransactionDataResult result = model.tradeWalletService.takerCreatesAndSignsDepositTx( + TradeWalletService.Result result = model.tradeWalletService.takerCreatesAndSignsDepositTx( takerInputAmount, msOutputAmount, model.offerer.connectedOutputsForAllInputs, diff --git a/core/src/main/resources/logback.xml b/core/src/main/resources/logback.xml index 3fae4c4690..fef8bec188 100644 --- a/core/src/main/resources/logback.xml +++ b/core/src/main/resources/logback.xml @@ -33,7 +33,7 @@ - +