Fix bug with randomly failing tx signing caused by sorting of pub keys

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();
+        pubKey = keyPair.getPubKey();
-        pubKeyHash = keyPair.getPubOnly().getPubKeyHash();
+        pubKeyHash = keyPair.getPubKeyHash();
     }
 
     public String getOfferId() {

core/src/main/java/io/bitsquare/btc/FeePolicy.java

@@ -55,8 +55,8 @@ public class FeePolicy {
                 takeOfferFeeAddress = "1BVxNn3T12veSK6DgqwU4Hdn7QHcDDRag7";
                 break;
             case REGTEST:
-                createOfferFeeAddress = "mwjWBMW3tcvSDQWooybzumY8RFm4BkKSxZ";
+                createOfferFeeAddress = "mxmKZruv9x9JLcEj6rZx6Hnm4LLAcQHtcr";
-                takeOfferFeeAddress = "mwjWBMW3tcvSDQWooybzumY8RFm4BkKSxZ";
+                takeOfferFeeAddress = "mxmKZruv9x9JLcEj6rZx6Hnm4LLAcQHtcr";
                 break;
             default:
                 throw new BitsquareException("Unknown bitcoin network: %s", bitcoinNetwork);

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.
+        // offererInputAmount includes the tx fee. So we subtract the fee to get the dummyOutputAmount.
-        Coin dummyOutputAmount = inputAmount.subtract(FeePolicy.TX_FEE);
+        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[0] dummyOutputAmount (offererInputAmount - tx fee)
-        OUT[1] Optional Change = inputAmount - dummyOutputAmount - 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<TransactionOutput> outputs = new ArrayList<>();
-        for (TransactionOutput output : dummyTX.getOutputs()) {
+        for (int i = 1; i < dummyTX.getOutputs().size(); i++) {
-            if (output.equals(dummyOutput))
+            outputs.add(dummyTX.getOutputs().get(i));
-                continue;
-            outputs.add(output);
         }
 
-        return new TransactionDataResult(connectedOutputsForAllInputs, outputs);
+        return new Result(connectedOutputsForAllInputs, outputs);
     }
 
-    public TransactionDataResult takerCreatesAndSignsDepositTx(Coin takerInputAmount,
+    public Result takerCreatesAndSignsDepositTx(Coin takerInputAmount,
-                                                               Coin msOutputAmount,
+                                                Coin msOutputAmount,
-                                                               List<TransactionOutput> offererConnectedOutputsForAllInputs,
+                                                List<TransactionOutput> offererConnectedOutputsForAllInputs,
-                                                               List<TransactionOutput> offererOutputs,
+                                                List<TransactionOutput> offererOutputs,
-                                                               AddressEntry takerAddressInfo,
+                                                AddressEntry takerAddressInfo,
-                                                               byte[] offererPubKey,
+                                                byte[] offererPubKey,
-                                                               byte[] takerPubKey,
+                                                byte[] takerPubKey,
-                                                               byte[] arbitratorPubKey) throws SigningException,
+                                                byte[] arbitratorPubKey) throws SigningException, TransactionVerificationException, WalletException {
-            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<TransactionOutput> connectedOutputsForAllTakerInputs = new ArrayList<>();
+        List<TransactionOutput> 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());
+        TransactionOutput p2SHMultiSigOutput = new TransactionOutput(params, preparedDepositTx, msOutputAmount, p2SHMultiSigOutputScript.getProgram());
-        preparedDepositTx.addOutput(msOutput);
+        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,
+    public void offererSignsAndPublishDepositTx(Transaction takersPreparedDepositTx,
-                                         List<TransactionOutput> offererConnectedOutputsForAllInputs,
+                                                List<TransactionOutput> offererConnectedOutputsForAllInputs,
-                                         List<TransactionOutput> takerConnectedOutputsForAllInputs,
+                                                List<TransactionOutput> takerConnectedOutputsForAllInputs,
-                                         List<TransactionOutput> offererOutputs,
+                                                List<TransactionOutput> offererOutputs,
-                                         Coin offererInputAmount,
+                                                Coin offererInputAmount,
-                                         byte[] offererPubKey,
+                                                byte[] offererPubKey,
-                                         byte[] takerPubKey,
+                                                byte[] takerPubKey,
-                                         byte[] arbitratorPubKey,
+                                                byte[] arbitratorPubKey,
-                                         FutureCallback<Transaction> callback) throws SigningException, TransactionVerificationException, WalletException {
+                                                FutureCallback<Transaction> 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);
+        Script p2SHMultiSigOutputScript = getP2SHMultiSigOutputScript(offererPubKey, takerPubKey, arbitratorPubKey);
-        if (!takersDepositTx.getOutput(0).getScriptPubKey().equals(multiSigOutputScript))
+        if (!takersPreparedDepositTx.getOutput(0).getScriptPubKey().equals(p2SHMultiSigOutputScript))
-            throw new TransactionVerificationException("Takers multiSigOutputScript does not match to my multiSigOutputScript");
+            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
+            // We grab the signature from the takersPreparedDepositTx and apply it to the new tx input
-            TransactionInput takerInput = takersDepositTx.getInputs().get(offererConnectedOutputsForAllInputs.size());
+            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
+        // Add all outputs from takersPreparedDepositTx to depositTx
-        for (TransactionOutput output : takersDepositTx.getOutputs()) {
+        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<Transaction> 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 r " + ECKey.ECDSASignature.decodeFromDER(offererSignature).r.toString());
-        log.trace("offererSignature s " + ECKey.ECDSASignature.decodeFromDER(offererSignature).toCanonicalised().s.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());
+        if (!Utils.HEX.encode(takerAddressEntry.getPubKey()).equals(Utils.HEX.encode(takerPubKey)))
-        // MS redeemScript
+            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) {
+    // Don't use ScriptBuilder.createRedeemScript and ScriptBuilder.createP2SHOutputScript as they use a sorting 
-        ECKey offererKey = ECKey.fromPublicOnly(offererPubKey);
+    // (Collections.sort(pubKeys, ECKey.PUBKEY_COMPARATOR);) which can lead to a non-matching list of signatures with pubKeys and the executeMultiSig does 
-        ECKey takerKey = ECKey.fromPublicOnly(takerPubKey);
+    // not iterate all possible combinations of sig/pubKeys leading to a verification fault. That nasty bug happens just randomly as the list after sorting 
-        ECKey arbitratorKey = ECKey.fromPublicOnly(arbitratorPubKey);
+    // might differ from the provided one or not.
-        List<ECKey> keys = ImmutableList.of(offererKey, takerKey, arbitratorKey);
+    // Changing the while loop in executeMultiSig to fix that does not help as the reference implementation seems to behave the same (not iterating all 
-        return ScriptBuilder.createP2SHOutputScript(2, keys);
+    // 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<ECKey> keys = ImmutableList.of(offererKey, takerKey, arbitratorKey);
+        // Take care of sorting!
-        return ScriptBuilder.createRedeemScript(2, keys);
+        List<ECKey> keys = ImmutableList.of(arbitratorKey, takerKey, offererKey);
+        return ScriptBuilder.createMultiSigOutputScript(2, keys);
     }
 
-    private Transaction createPayoutTx(Transaction depositTx, Coin offererPayoutAmount, Coin takerPayoutAmount,
+    private Script getP2SHMultiSigOutputScript(byte[] offererPubKey, byte[] takerPubKey, byte[] arbitratorPubKey) {
-                                       String offererAddressString, String takerAddressString) throws AddressFormatException {
+        return ScriptBuilder.createP2SHOutputScript(getMultiSigRedeemScript(offererPubKey, takerPubKey, arbitratorPubKey));
+    }
 
-        TransactionOutput multiSigOutput = depositTx.getOutput(0);
+    private Transaction createPayoutTx(Transaction depositTx,
-        Transaction tx = new Transaction(params);
+                                       Coin offererPayoutAmount,
-        tx.addInput(multiSigOutput);
+                                       Coin takerPayoutAmount,
-        tx.addOutput(offererPayoutAmount, new Address(params, offererAddressString));
+                                       String offererAddressString,
-        tx.addOutput(takerPayoutAmount, new Address(params, takerAddressString));
+                                       String takerAddressString) throws AddressFormatException {
-        return tx;
+
+        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<TransactionOutput> connectedOutputsForAllInputs;
         private List<TransactionOutput> outputs;
         private Transaction depositTx;
@@ -596,12 +631,12 @@ public class TradeWalletService {
 
         private byte[] offererSignature;
 
-        public TransactionDataResult(List<TransactionOutput> connectedOutputsForAllInputs, List<TransactionOutput> outputs) {
+        public Result(List<TransactionOutput> connectedOutputsForAllInputs, List<TransactionOutput> outputs) {
             this.connectedOutputsForAllInputs = connectedOutputsForAllInputs;
             this.outputs = outputs;
         }
 
-        public TransactionDataResult(Transaction depositTx, List<TransactionOutput> connectedOutputsForAllInputs, List<TransactionOutput> outputs) {
+        public Result(Transaction depositTx, List<TransactionOutput> connectedOutputsForAllInputs, List<TransactionOutput> outputs) {
             this.depositTx = depositTx;
             this.connectedOutputsForAllInputs = connectedOutputsForAllInputs;
             this.outputs = outputs;

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<AddressConfidenceListener> addressConfidenceListeners = new CopyOnWriteArrayList<>();
     private final List<TxConfidenceListener> txConfidenceListeners = new CopyOnWriteArrayList<>();
     private final List<BalanceListener> balanceListeners = new CopyOnWriteArrayList<>();
-    private final ReentrantLock lock = Threading.lock(LOCK_NAME);
 
     private final ObservableDownloadListener downloadListener = new ObservableDownloadListener();
     private final Observable<Double> downloadProgress = downloadListener.getObservable();
@@ -108,7 +103,7 @@ public class WalletService {
     private Wallet wallet;
     private AddressEntry registrationAddressEntry;
     private AddressEntry arbitratorDepositAddressEntry;
-    private @GuardedBy(LOCK_NAME) List<AddressEntry> addressEntryList = new ArrayList<>();
+    private List<AddressEntry> 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,8 +293,7 @@ public class WalletService {
 
     public AddressEntry getAddressEntry(String offerId) {
         log.trace("getAddressEntry called with offerId " + offerId);
-        Optional<AddressEntry> addressEntry = getAddressEntryList().stream().filter(e ->
+        Optional<AddressEntry> addressEntry = getAddressEntryList().stream().filter(e -> offerId.equals(e.getOfferId())).findFirst();
-                offerId.equals(e.getOfferId())).findFirst();
        
         if (addressEntry.isPresent())
             return addressEntry.get();
@@ -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();
+        persistence.write(this, "addressEntryList", addressEntryList);
-        try {
-            persistence.write(this, "addressEntryList", addressEntryList);
-        } finally {
-            lock.unlock();
-        }
     }
 
     private static void printTxWithInputs(String tracePrefix, Transaction tx) {

core/src/main/java/io/bitsquare/common/taskrunner/SharedTaskModel.java

@@ -25,4 +25,7 @@ public class SharedTaskModel {
 
     public void persist() {
     }
+
+    public void onComplete() {
+    }
 }

core/src/main/java/io/bitsquare/common/taskrunner/TaskRunner.java

@@ -77,14 +77,10 @@ public class TaskRunner<T extends SharedTaskModel> {
 
     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);

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,
 

core/src/main/java/io/bitsquare/gui/main/portfolio/pending/PendingTradesView.java

@@ -107,7 +107,7 @@ public class PendingTradesView extends ActivatableViewAndModel<AnchorPane, Pendi
         setDateColumnCellFactory();
 
         //TODO just temp for testing
-        withdrawAddressTextField.setText("mwjWBMW3tcvSDQWooybzumY8RFm4BkKSxZ");
+        withdrawAddressTextField.setText("mxmKZruv9x9JLcEj6rZx6Hnm4LLAcQHtcr");
 
         table.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
         table.setPlaceholder(new Label("No pending trades available"));

core/src/main/java/io/bitsquare/persistence/Persistence.java

@@ -126,6 +126,16 @@ public class Persistence {
         write(classInstance.getClass().getName() + "." + propertyKey, value);
     }
 
+    public void remove(Object classInstance, String propertyKey) {
+        try {
+            lock.lock();
+            rootMap.remove(classInstance.getClass().getName() + "." + propertyKey);
+            saveObjectToFile((Serializable) rootMap);
+        } finally {
+            lock.unlock();
+        }
+    }
+
     public void write(Object classInstance, Serializable value) {
         write(classInstance.getClass().getName(), value);
     }

core/src/main/java/io/bitsquare/trade/protocol/trade/messages/RequestOffererPublishDepositTxMessage.java

@@ -36,8 +36,8 @@ public class RequestOffererPublishDepositTxMessage extends TradeMessage implemen
     public final PublicKey takerMessagePublicKey;
     public final String takerContractAsJson;
     public final String takerContractSignature;
-    public final String takerPayoutAddress;
+    public final String takerPayoutAddressString;
-    public final Transaction takersDepositTx;
+    public final Transaction takersPreparedDepositTx;
     public final List<TransactionOutput> takerConnectedOutputsForAllInputs;
     public final List<TransactionOutput> takerOutputs;
 
@@ -47,8 +47,8 @@ public class RequestOffererPublishDepositTxMessage extends TradeMessage implemen
                                                  PublicKey takerMessagePublicKey,
                                                  String takerContractAsJson,
                                                  String takerContractSignature,
-                                                 String takerPayoutAddress,
+                                                 String takerPayoutAddressString,
-                                                 Transaction takersDepositTx,
+                                                 Transaction takersPreparedDepositTx,
                                                  List<TransactionOutput> takerConnectedOutputsForAllInputs,
                                                  List<TransactionOutput> 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.takerPayoutAddressString = takerPayoutAddressString;
-        this.takersDepositTx = takersDepositTx;
+        this.takersPreparedDepositTx = takersPreparedDepositTx;
         this.takerConnectedOutputsForAllInputs = takerConnectedOutputsForAllInputs;
         this.takerOutputs = takerOutputs;
     }

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<BuyerAsOffererModel> taskRunner = new BuyerAsOffererTaskRunner<>(model,
                 () -> {
                     log.debug("sequence at handlePayoutTxPublishedMessage completed");
+                   
+                    // we are done!
+                    model.onComplete();
                 },
                 (errorMessage) -> {
                     log.error(errorMessage);

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() {
+    @Override
-        return publishedDepositTx;
+    public void onComplete() {
-    }
+        // Just in case of successful completion we delete our persisted object
-
+        persistence.remove(this, "BuyerAsOffererModel_" + id);
-    public void setPublishedDepositTx(Transaction publishedDepositTx) {
-        this.publishedDepositTx = publishedDepositTx;
     }
 
     public String getTakeOfferFeeTxId() {

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<TransactionOutput> connectedOutputsForAllInputs;
     public String payoutAddressString;
     public byte[] pubKey;

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<BuyerAsOffererModel> {
+public class CreateAndSignPayoutTx extends Task<BuyerAsOffererModel> {
-    private static final Logger log = LoggerFactory.getLogger(SignPayoutTx.class);
+    private static final Logger log = LoggerFactory.getLogger(CreateAndSignPayoutTx.class);
 
-    public SignPayoutTx(TaskRunner taskHandler, BuyerAsOffererModel model) {
+    public CreateAndSignPayoutTx(TaskRunner taskHandler, BuyerAsOffererModel model) {
         super(taskHandler, model);
     }
 

core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/CreateOffererDepositTxInputs.java

@@ -39,7 +39,7 @@ public class CreateOffererDepositTxInputs extends Task<BuyerAsOffererModel> {
     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();

core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/ProcessRequestOffererPublishDepositTxMessage.java

@@ -46,8 +46,8 @@ public class ProcessRequestOffererPublishDepositTxMessage extends Task<BuyerAsOf
             model.taker.messagePublicKey = checkNotNull(message.takerMessagePublicKey);
             model.taker.contractAsJson = nonEmptyStringOf(message.takerContractAsJson);
             model.taker.contractSignature = nonEmptyStringOf(message.takerContractSignature);
-            model.taker.payoutAddressString = nonEmptyStringOf(message.takerPayoutAddress);
+            model.taker.payoutAddressString = nonEmptyStringOf(message.takerPayoutAddressString);
-            model.taker.depositTx = checkNotNull(message.takersDepositTx);
+            model.taker.preparedDepositTx = checkNotNull(message.takersPreparedDepositTx);
             model.taker.connectedOutputsForAllInputs = checkNotNull(message.takerConnectedOutputsForAllInputs);
             checkArgument(message.takerConnectedOutputsForAllInputs.size() > 0);
 

core/src/main/java/io/bitsquare/trade/protocol/trade/offerer/tasks/SignAndPublishDepositTx.java

@@ -44,8 +44,8 @@ public class SignAndPublishDepositTx extends Task<BuyerAsOffererModel> {
     protected void doRun() {
         try {
             Coin offererInputAmount = model.trade.getSecurityDeposit().add(FeePolicy.TX_FEE);
-            model.tradeWalletService.offererSignsAndPublishTx(
+            model.tradeWalletService.offererSignsAndPublishDepositTx(
-                    model.taker.depositTx,
+                    model.taker.preparedDepositTx,
                     model.offerer.connectedOutputsForAllInputs,
                     model.taker.connectedOutputsForAllInputs,
                     model.offerer.outputs,
@@ -58,7 +58,6 @@ public class SignAndPublishDepositTx extends Task<BuyerAsOffererModel> {
                         public void onSuccess(Transaction transaction) {
                             log.trace("offererSignAndPublishTx succeeded " + transaction);
 
-                            model.setPublishedDepositTx(transaction);
                             model.trade.setDepositTx(transaction);
                             model.trade.setState(Trade.State.DEPOSIT_PUBLISHED);
 

core/src/main/java/io/bitsquare/trade/protocol/trade/taker/SellerAsTakerProtocol.java

@@ -166,6 +166,9 @@ public class SellerAsTakerProtocol {
         SellerAsTakerTaskRunner<SellerAsTakerModel> 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
     ///////////////////////////////////////////////////////////////////////////////////////////

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;
     }

core/src/main/java/io/bitsquare/trade/protocol/trade/taker/tasks/TakerCreatesAndSignsDepositTx.java

@@ -42,7 +42,7 @@ public class TakerCreatesAndSignsDepositTx extends Task<SellerAsTakerModel> {
             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,

core/src/main/resources/logback.xml

@@ -33,7 +33,7 @@
   <!--  <logger name="io.bitsquare.persistence.Persistence" level="ERROR"/>-->
     <logger name="io.bitsquare.locale.BSResources" level="ERROR"/>
 
-    <logger name="org.bitcoinj" level="DEBUG"/>
+    <logger name="org.bitcoinj" level="TRACE"/>
     <logger name="net.tomp2p" level="ERROR"/>
 
     <logger name="org.bitcoinj.core.BitcoinSerializer" level="WARN"/>