various fixes to psbt serialisation and wallet tx creation

src/main/java/com/sparrowwallet/drongo/KeyDerivation.java

@@ -9,7 +9,7 @@ import java.util.List;
 public class KeyDerivation {
     private final String masterFingerprint;
     private final String derivationPath;
-    private final transient List<ChildNumber> derivation;
+    private transient List<ChildNumber> derivation;
 
     public KeyDerivation(String masterFingerprint, String derivationPath) {
         this.masterFingerprint = masterFingerprint == null ? null : masterFingerprint.toLowerCase();
@@ -26,12 +26,20 @@ public class KeyDerivation {
     }
 
     public List<ChildNumber> getDerivation() {
+        if(derivation == null) {
+            derivation = parsePath(derivationPath);
+        }
+
         return Collections.unmodifiableList(derivation);
     }
 
-    public KeyDerivation extend(ChildNumber childNumber) {
+    public KeyDerivation extend(ChildNumber extension) {
-        List<ChildNumber> extendedDerivation = new ArrayList<>(derivation);
+        return extend(List.of(extension));
-        extendedDerivation.add(childNumber);
+    }
+
+    public KeyDerivation extend(List<ChildNumber> extension) {
+        List<ChildNumber> extendedDerivation = new ArrayList<>(getDerivation());
+        extendedDerivation.addAll(extension);
         return new KeyDerivation(masterFingerprint, writePath(extendedDerivation));
     }
 

src/main/java/com/sparrowwallet/drongo/protocol/Transaction.java

@@ -146,17 +146,32 @@ public class Transaction extends ChildMessage {
         this.segwitVersion = segwitVersion;
     }
 
+    public void clearSegwit() {
+        if(segwit) {
+            adjustLength(-2);
+            segwit = false;
+        }
+    }
+
     public boolean hasWitnesses() {
-        for (TransactionInput in : inputs)
+        for(TransactionInput in : inputs) {
-            if (in.hasWitness())
+            if(in.hasWitness()) {
                 return true;
+            }
+        }
+
         return false;
     }
 
     public byte[] bitcoinSerialize() {
+        boolean useWitnessFormat = isSegwit();
+        return bitcoinSerialize(useWitnessFormat);
+    }
+
+    public byte[] bitcoinSerialize(boolean useWitnessFormat) {
         try {
             ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-            bitcoinSerializeToStream(outputStream);
+            bitcoinSerializeToStream(outputStream, useWitnessFormat);
             return outputStream.toByteArray();
         } catch (IOException e) {
             //can't happen
@@ -166,8 +181,8 @@ public class Transaction extends ChildMessage {
     }
 
     public void bitcoinSerializeToStream(OutputStream stream) throws IOException {
-        boolean useSegwit = isSegwit();
+        boolean useWitnessFormat = isSegwit();
-        bitcoinSerializeToStream(stream, useSegwit);
+        bitcoinSerializeToStream(stream, useWitnessFormat);
     }
 
     /**
@@ -175,30 +190,40 @@ public class Transaction extends ChildMessage {
      * <a href="https://en.bitcoin.it/wiki/Protocol_documentation#tx">classic format</a>, depending on if segwit is
      * desired.
      */
-    protected void bitcoinSerializeToStream(OutputStream stream, boolean useSegwit) throws IOException {
+    protected void bitcoinSerializeToStream(OutputStream stream, boolean useWitnessFormat) throws IOException {
         // version
         uint32ToByteStreamLE(version, stream);
+
         // marker, flag
-        if (useSegwit) {
+        if(useWitnessFormat) {
             stream.write(0);
             stream.write(segwitVersion);
         }
+
         // txin_count, txins
         stream.write(new VarInt(inputs.size()).encode());
-        for (TransactionInput in : inputs)
+        for(TransactionInput in : inputs) {
             in.bitcoinSerializeToStream(stream);
+        }
+
         // txout_count, txouts
         stream.write(new VarInt(outputs.size()).encode());
-        for (TransactionOutput out : outputs)
+        for(TransactionOutput out : outputs) {
             out.bitcoinSerializeToStream(stream);
+        }
+
         // script_witnesses
-        if (useSegwit) {
+        if(useWitnessFormat) {
-            for (TransactionInput in : inputs) {
+            for(TransactionInput in : inputs) {
-                if (in.hasWitness()) {
+                //Per BIP141 all txins must have a witness
-                    in.getWitness().bitcoinSerializeToStream(stream);
+                if(!in.hasWitness()) {
+                    in.setWitness(new TransactionWitness(this));
                 }
+
+                in.getWitness().bitcoinSerializeToStream(stream);
             }
         }
+
         // lock_time
         uint32ToByteStreamLE(locktime, stream);
     }
@@ -333,6 +358,10 @@ public class Transaction extends ChildMessage {
         return Collections.unmodifiableList(outputs);
     }
 
+    public void shuffleOutputs() {
+        Collections.shuffle(outputs);
+    }
+
     public TransactionOutput addOutput(long value, Script script) {
         return addOutput(new TransactionOutput(this, value, script));
     }
@@ -401,11 +430,11 @@ public class Transaction extends ChildMessage {
         return version > 0 && version < 5;
     }
 
-    public Sha256Hash hashForSignature(int inputIndex, Script redeemScript, SigHash sigHash) {
+    public Sha256Hash hashForLegacySignature(int inputIndex, Script redeemScript, SigHash sigHash) {
-        return hashForSignature(inputIndex, redeemScript.getProgram(), sigHash.value);
+        return hashForLegacySignature(inputIndex, redeemScript.getProgram(), sigHash.value);
     }
 
-    public Sha256Hash hashForSignature(int inputIndex, byte[] connectedScript, byte sigHashType) {
+    public Sha256Hash hashForLegacySignature(int inputIndex, byte[] connectedScript, byte sigHashType) {
         try {
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             this.bitcoinSerializeToStream(baos);
@@ -417,7 +446,6 @@ public class Transaction extends ChildMessage {
             for (int i = 0; i < tx.inputs.size(); i++) {
                 TransactionInput input = tx.inputs.get(i);
                 input.clearScriptBytes();
-                input.clearWitness();
             }
 
             // This step has no purpose beyond being synchronized with Bitcoin Core's bugs. OP_CODESEPARATOR
@@ -432,16 +460,18 @@ public class Transaction extends ChildMessage {
             TransactionInput input = tx.inputs.get(inputIndex);
             input.setScriptBytes(connectedScript);
 
-            if ((sigHashType & 0x1f) == SigHash.NONE.value) {
+            if((sigHashType & 0x1f) == SigHash.NONE.value) {
                 // SIGHASH_NONE means no outputs are signed at all - the signature is effectively for a "blank cheque".
                 tx.outputs = new ArrayList<>(0);
                 // The signature isn't broken by new versions of the transaction issued by other parties.
-                for (int i = 0; i < tx.inputs.size(); i++)
+                for(int i = 0; i < tx.inputs.size(); i++) {
-                    if (i != inputIndex)
+                    if(i != inputIndex) {
                         tx.inputs.get(i).setSequenceNumber(0);
-            } else if ((sigHashType & 0x1f) == SigHash.SINGLE.value) {
+                    }
+                }
+            } else if((sigHashType & 0x1f) == SigHash.SINGLE.value) {
                 // SIGHASH_SINGLE means only sign the output at the same index as the input (ie, my output).
-                if (inputIndex >= tx.outputs.size()) {
+                if(inputIndex >= tx.outputs.size()) {
                     // The input index is beyond the number of outputs, it's a buggy signature made by a broken
                     // Bitcoin implementation. Bitcoin Core also contains a bug in handling this case:
                     // any transaction output that is signed in this case will result in both the signed output
@@ -455,15 +485,18 @@ public class Transaction extends ChildMessage {
                 // In SIGHASH_SINGLE the outputs after the matching input index are deleted, and the outputs before
                 // that position are "nulled out". Unintuitively, the value in a "null" transaction is set to -1.
                 tx.outputs = new ArrayList<>(tx.outputs.subList(0, inputIndex + 1));
-                for (int i = 0; i < inputIndex; i++)
+                for(int i = 0; i < inputIndex; i++) {
-                    tx.outputs.set(i, new TransactionOutput(tx, -1L, new byte[] {}));
+                    tx.outputs.set(i, new TransactionOutput(tx, -1L, new byte[]{}));
+                }
                 // The signature isn't broken by new versions of the transaction issued by other parties.
-                for (int i = 0; i < tx.inputs.size(); i++)
+                for(int i = 0; i < tx.inputs.size(); i++) {
-                    if (i != inputIndex)
+                    if(i != inputIndex) {
                         tx.inputs.get(i).setSequenceNumber(0);
+                    }
+                }
             }
 
-            if ((sigHashType & SigHash.ANYONECANPAY.value) == SigHash.ANYONECANPAY.value) {
+            if((sigHashType & SigHash.ANYONECANPAY.value) == SigHash.ANYONECANPAY.value) {
                 // SIGHASH_ANYONECANPAY means the signature in the input is not broken by changes/additions/removals
                 // of other inputs. For example, this is useful for building assurance contracts.
                 tx.inputs = new ArrayList<>();
@@ -510,38 +543,39 @@ public class Transaction extends ChildMessage {
             boolean anyoneCanPay = (sigHashType & SigHash.ANYONECANPAY.value) == SigHash.ANYONECANPAY.value;
             boolean signAll = (basicSigHashType != SigHash.SINGLE.value) && (basicSigHashType != SigHash.NONE.value);
 
-            if (!anyoneCanPay) {
+            if(!anyoneCanPay) {
                 ByteArrayOutputStream bosHashPrevouts = new UnsafeByteArrayOutputStream(256);
-                for (int i = 0; i < this.inputs.size(); ++i) {
+                for(int i = 0; i < this.inputs.size(); ++i) {
                     bosHashPrevouts.write(this.inputs.get(i).getOutpoint().getHash().getReversedBytes());
                     uint32ToByteStreamLE(this.inputs.get(i).getOutpoint().getIndex(), bosHashPrevouts);
                 }
                 hashPrevouts = Sha256Hash.hashTwice(bosHashPrevouts.toByteArray());
             }
 
-            if (!anyoneCanPay && signAll) {
+            if(!anyoneCanPay && signAll) {
                 ByteArrayOutputStream bosSequence = new UnsafeByteArrayOutputStream(256);
-                for (int i = 0; i < this.inputs.size(); ++i) {
+                for(int i = 0; i < this.inputs.size(); ++i) {
                     uint32ToByteStreamLE(this.inputs.get(i).getSequenceNumber(), bosSequence);
                 }
                 hashSequence = Sha256Hash.hashTwice(bosSequence.toByteArray());
             }
 
-            if (signAll) {
+            if(signAll) {
                 ByteArrayOutputStream bosHashOutputs = new UnsafeByteArrayOutputStream(256);
-                for (int i = 0; i < this.outputs.size(); ++i) {
+                for(int i = 0; i < this.outputs.size(); ++i) {
                     uint64ToByteStreamLE(BigInteger.valueOf(this.outputs.get(i).getValue()), bosHashOutputs);
                     bosHashOutputs.write(new VarInt(this.outputs.get(i).getScriptBytes().length).encode());
                     bosHashOutputs.write(this.outputs.get(i).getScriptBytes());
                 }
                 hashOutputs = Sha256Hash.hashTwice(bosHashOutputs.toByteArray());
-            } else if (basicSigHashType == SigHash.SINGLE.value && inputIndex < outputs.size()) {
+            } else if(basicSigHashType == SigHash.SINGLE.value && inputIndex < outputs.size()) {
                 ByteArrayOutputStream bosHashOutputs = new UnsafeByteArrayOutputStream(256);
                 uint64ToByteStreamLE(BigInteger.valueOf(this.outputs.get(inputIndex).getValue()), bosHashOutputs);
                 bosHashOutputs.write(new VarInt(this.outputs.get(inputIndex).getScriptBytes().length).encode());
                 bosHashOutputs.write(this.outputs.get(inputIndex).getScriptBytes());
                 hashOutputs = Sha256Hash.hashTwice(bosHashOutputs.toByteArray());
             }
+
             uint32ToByteStreamLE(version, bos);
             bos.write(hashPrevouts);
             bos.write(hashSequence);

src/main/java/com/sparrowwallet/drongo/protocol/TransactionInput.java

@@ -110,10 +110,6 @@ public class TransactionInput extends ChildMessage {
         this.witness = witness;
     }
 
-    public void clearWitness() {
-        setWitness(null);
-    }
-
     public boolean hasWitness() {
         return witness != null;
     }

src/main/java/com/sparrowwallet/drongo/protocol/TransactionOutput.java

@@ -8,13 +8,8 @@ import java.io.IOException;
 import java.io.OutputStream;
 
 public class TransactionOutput extends ChildMessage {
-    // The output's value is kept as a native type in order to save class instances.
     private long value;
-
-    // A transaction output has a script used for authenticating that the redeemer is allowed to spend
-    // this output.
     private byte[] scriptBytes;
-
     private Script script;
 
     private Address[] addresses = new Address[0];

src/main/java/com/sparrowwallet/drongo/psbt/PSBT.java

@@ -3,6 +3,7 @@ package com.sparrowwallet.drongo.psbt;
 import com.sparrowwallet.drongo.ExtendedKey;
 import com.sparrowwallet.drongo.KeyDerivation;
 import com.sparrowwallet.drongo.Utils;
+import com.sparrowwallet.drongo.address.Address;
 import com.sparrowwallet.drongo.crypto.ECKey;
 import com.sparrowwallet.drongo.protocol.*;
 import com.sparrowwallet.drongo.wallet.*;
@@ -58,17 +59,33 @@ public class PSBT {
     }
 
     public PSBT(WalletTransaction walletTransaction) {
+        this(walletTransaction, null, true);
+    }
+
+    public PSBT(WalletTransaction walletTransaction, Integer version, boolean includeGlobalXpubs) {
         Wallet wallet = walletTransaction.getWallet();
 
         transaction = new Transaction(walletTransaction.getTransaction().bitcoinSerialize());
+
+        //Clear segwit marker & flag, scriptSigs and all witness data as per BIP174
+        transaction.clearSegwit();
         for(TransactionInput input : transaction.getInputs()) {
             input.clearScriptBytes();
-            input.clearWitness();
+            input.setWitness(null);
         }
-        for(Keystore keystore : walletTransaction.getWallet().getKeystores()) {
+
-            extendedPublicKeys.put(keystore.getExtendedPublicKey(), keystore.getKeyDerivation());
+        //Shuffle outputs so change outputs are less obvious
+        transaction.shuffleOutputs();
+
+        if(includeGlobalXpubs) {
+            for(Keystore keystore : walletTransaction.getWallet().getKeystores()) {
+                extendedPublicKeys.put(keystore.getExtendedPublicKey(), keystore.getKeyDerivation());
+            }
+        }
+
+        if(version != null) {
+            this.version = version;
         }
-        version = 0;
 
         int inputIndex = 0;
         for(Iterator<Map.Entry<BlockTransactionHashIndex, WalletNode>> iter = walletTransaction.getSelectedUtxos().entrySet().iterator(); iter.hasNext(); inputIndex++) {
@@ -92,7 +109,7 @@ public class PSBT {
             Map<ECKey, KeyDerivation> derivedPublicKeys = new LinkedHashMap<>();
             for(Keystore keystore : wallet.getKeystores()) {
                 WalletNode walletNode = utxoEntry.getValue();
-                derivedPublicKeys.put(keystore.getPubKey(walletNode), keystore.getKeyDerivation());
+                derivedPublicKeys.put(keystore.getPubKey(walletNode), keystore.getKeyDerivation().extend(walletNode.getDerivation()));
             }
 
             PSBTInput psbtInput = new PSBTInput(wallet.getScriptType(), transaction, inputIndex, utxo, utxoIndex, redeemScript, witnessScript, derivedPublicKeys, Collections.emptyMap());
@@ -100,8 +117,19 @@ public class PSBT {
         }
 
         List<WalletNode> outputNodes = new ArrayList<>();
-        outputNodes.add(wallet.getWalletAddresses().getOrDefault(walletTransaction.getRecipientAddress(), null));
+        for(TransactionOutput txOutput : transaction.getOutputs()) {
-        outputNodes.add(walletTransaction.getChangeNode());
+            try {
+                Address address = txOutput.getScript().getToAddresses()[0];
+                if(address.equals(walletTransaction.getRecipientAddress())) {
+                    outputNodes.add(wallet.getWalletAddresses().getOrDefault(walletTransaction.getRecipientAddress(), null));
+                } else if(address.equals(wallet.getAddress(walletTransaction.getChangeNode()))) {
+                    outputNodes.add(walletTransaction.getChangeNode());
+                }
+            } catch(NonStandardScriptException e) {
+                //Should never happen
+                throw new IllegalArgumentException(e);
+            }
+        }
 
         for(int outputIndex = 0; outputIndex < outputNodes.size(); outputIndex++) {
             WalletNode outputNode = outputNodes.get(outputIndex);
@@ -109,7 +137,7 @@ public class PSBT {
                 PSBTOutput externalRecipientOutput = new PSBTOutput(null, null, Collections.emptyMap(), Collections.emptyMap());
                 psbtOutputs.add(externalRecipientOutput);
             } else {
-                TransactionOutput txOutput = walletTransaction.getTransaction().getOutputs().get(outputIndex);
+                TransactionOutput txOutput = transaction.getOutputs().get(outputIndex);
 
                 //Construct dummy transaction to spend the UTXO created by this wallet's txOutput
                 Transaction transaction = new Transaction();
@@ -127,7 +155,7 @@ public class PSBT {
 
                 Map<ECKey, KeyDerivation> derivedPublicKeys = new LinkedHashMap<>();
                 for(Keystore keystore : wallet.getKeystores()) {
-                    derivedPublicKeys.put(keystore.getPubKey(outputNode), keystore.getKeyDerivation());
+                    derivedPublicKeys.put(keystore.getPubKey(outputNode), keystore.getKeyDerivation().extend(outputNode.getDerivation()));
                 }
 
                 PSBTOutput walletOutput = new PSBTOutput(redeemScript, witnessScript, derivedPublicKeys, Collections.emptyMap());
@@ -369,7 +397,7 @@ public class PSBT {
         List<PSBTEntry> entries = new ArrayList<>();
 
         if(transaction != null) {
-            entries.add(populateEntry(PSBT_GLOBAL_UNSIGNED_TX, null, transaction.bitcoinSerialize()));
+            entries.add(populateEntry(PSBT_GLOBAL_UNSIGNED_TX, null, transaction.bitcoinSerialize(false)));
         }
 
         for(Map.Entry<ExtendedKey, KeyDerivation> entry : extendedPublicKeys.entrySet()) {

src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java

@@ -496,7 +496,7 @@ public class PSBTInput {
             long prevValue = getWitnessUtxo().getValue();
             hash = transaction.hashForWitnessSignature(index, connectedScript, prevValue, localSigHash);
         } else {
-            hash = transaction.hashForSignature(index, connectedScript, localSigHash);
+            hash = transaction.hashForLegacySignature(index, connectedScript, localSigHash);
         }
 
         return hash;

src/main/java/com/sparrowwallet/drongo/wallet/Wallet.java

@@ -412,19 +412,27 @@ public class Wallet {
         return getFee(changeOutput, feeRate, longTermFeeRate);
     }
 
-    public WalletTransaction createWalletTransaction(List<UtxoSelector> utxoSelectors, Address recipientAddress, long recipientAmount, double feeRate, double longTermFeeRate, Long fee, boolean sendAll, boolean groupByAddress, boolean includeMempoolChange) throws InsufficientFundsException {
+    public WalletTransaction createWalletTransaction(List<UtxoSelector> utxoSelectors, Address recipientAddress, long recipientAmount, double feeRate, double longTermFeeRate, Long fee, Integer currentBlockHeight, boolean sendAll, boolean groupByAddress, boolean includeMempoolChange) throws InsufficientFundsException {
         long valueRequiredAmt = recipientAmount;
 
         while(true) {
             Map<BlockTransactionHashIndex, WalletNode> selectedUtxos = selectInputs(utxoSelectors, valueRequiredAmt, feeRate, longTermFeeRate, groupByAddress, includeMempoolChange);
             long totalSelectedAmt = selectedUtxos.keySet().stream().mapToLong(BlockTransactionHashIndex::getValue).sum();
 
-            //Add inputs
             Transaction transaction = new Transaction();
+            transaction.setVersion(2);
+            if(currentBlockHeight != null) {
+                transaction.setLocktime(currentBlockHeight.longValue());
+            }
+
+            //Add inputs
             for(Map.Entry<BlockTransactionHashIndex, WalletNode> selectedUtxo : selectedUtxos.entrySet()) {
                 Transaction prevTx = getTransactions().get(selectedUtxo.getKey().getHash()).getTransaction();
                 TransactionOutput prevTxOut = prevTx.getOutputs().get((int)selectedUtxo.getKey().getIndex());
-                addDummySpendingInput(transaction, selectedUtxo.getValue(), prevTxOut);
+                TransactionInput txInput = addDummySpendingInput(transaction, selectedUtxo.getValue(), prevTxOut);
+
+                //Enable opt-in RBF by default, matching Bitcoin Core and Electrum
+                txInput.setSequenceNumber(TransactionInput.SEQUENCE_RBF_ENABLED);
             }
 
             //Add recipient output