From 15beeefcb687c77adddbfe5e2b74c75b2c6f2196 Mon Sep 17 00:00:00 2001 From: Craig Raw Date: Sun, 26 Jul 2020 14:02:47 +0200 Subject: [PATCH] various fixes to psbt serialisation and wallet tx creation --- .../sparrowwallet/drongo/KeyDerivation.java | 16 ++- .../drongo/protocol/Transaction.java | 102 ++++++++++++------ .../drongo/protocol/TransactionInput.java | 4 - .../drongo/protocol/TransactionOutput.java | 5 - .../com/sparrowwallet/drongo/psbt/PSBT.java | 48 +++++++-- .../sparrowwallet/drongo/psbt/PSBTInput.java | 2 +- .../sparrowwallet/drongo/wallet/Wallet.java | 14 ++- 7 files changed, 130 insertions(+), 61 deletions(-) diff --git a/src/main/java/com/sparrowwallet/drongo/KeyDerivation.java b/src/main/java/com/sparrowwallet/drongo/KeyDerivation.java index a292b36..b1f4e10 100644 --- a/src/main/java/com/sparrowwallet/drongo/KeyDerivation.java +++ b/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 derivation; + private transient List derivation; public KeyDerivation(String masterFingerprint, String derivationPath) { this.masterFingerprint = masterFingerprint == null ? null : masterFingerprint.toLowerCase(); @@ -26,12 +26,20 @@ public class KeyDerivation { } public List getDerivation() { + if(derivation == null) { + derivation = parsePath(derivationPath); + } + return Collections.unmodifiableList(derivation); } - public KeyDerivation extend(ChildNumber childNumber) { - List extendedDerivation = new ArrayList<>(derivation); - extendedDerivation.add(childNumber); + public KeyDerivation extend(ChildNumber extension) { + return extend(List.of(extension)); + } + + public KeyDerivation extend(List extension) { + List extendedDerivation = new ArrayList<>(getDerivation()); + extendedDerivation.addAll(extension); return new KeyDerivation(masterFingerprint, writePath(extendedDerivation)); } diff --git a/src/main/java/com/sparrowwallet/drongo/protocol/Transaction.java b/src/main/java/com/sparrowwallet/drongo/protocol/Transaction.java index ee0f076..e37caeb 100644 --- a/src/main/java/com/sparrowwallet/drongo/protocol/Transaction.java +++ b/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) - if (in.hasWitness()) + for(TransactionInput in : inputs) { + 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(); - bitcoinSerializeToStream(stream, useSegwit); + boolean useWitnessFormat = isSegwit(); + bitcoinSerializeToStream(stream, useWitnessFormat); } /** @@ -175,30 +190,40 @@ public class Transaction extends ChildMessage { * classic format, 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) { - for (TransactionInput in : inputs) { - if (in.hasWitness()) { - in.getWitness().bitcoinSerializeToStream(stream); + if(useWitnessFormat) { + for(TransactionInput in : inputs) { + //Per BIP141 all txins must have a witness + 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) { - return hashForSignature(inputIndex, redeemScript.getProgram(), sigHash.value); + public Sha256Hash hashForLegacySignature(int inputIndex, Script redeemScript, SigHash sigHash) { + 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++) - if (i != inputIndex) + for(int i = 0; i < tx.inputs.size(); i++) { + 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++) - tx.outputs.set(i, new TransactionOutput(tx, -1L, new byte[] {})); + for(int i = 0; i < inputIndex; i++) { + 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++) - if (i != inputIndex) + for(int i = 0; i < tx.inputs.size(); i++) { + 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); diff --git a/src/main/java/com/sparrowwallet/drongo/protocol/TransactionInput.java b/src/main/java/com/sparrowwallet/drongo/protocol/TransactionInput.java index 8416ed0..1d43d3b 100644 --- a/src/main/java/com/sparrowwallet/drongo/protocol/TransactionInput.java +++ b/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; } diff --git a/src/main/java/com/sparrowwallet/drongo/protocol/TransactionOutput.java b/src/main/java/com/sparrowwallet/drongo/protocol/TransactionOutput.java index d4519d7..4645d01 100644 --- a/src/main/java/com/sparrowwallet/drongo/protocol/TransactionOutput.java +++ b/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]; diff --git a/src/main/java/com/sparrowwallet/drongo/psbt/PSBT.java b/src/main/java/com/sparrowwallet/drongo/psbt/PSBT.java index cde1c38..daf348f 100644 --- a/src/main/java/com/sparrowwallet/drongo/psbt/PSBT.java +++ b/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> iter = walletTransaction.getSelectedUtxos().entrySet().iterator(); iter.hasNext(); inputIndex++) { @@ -92,7 +109,7 @@ public class PSBT { Map 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 outputNodes = new ArrayList<>(); - outputNodes.add(wallet.getWalletAddresses().getOrDefault(walletTransaction.getRecipientAddress(), null)); - outputNodes.add(walletTransaction.getChangeNode()); + for(TransactionOutput txOutput : transaction.getOutputs()) { + 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 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 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 entry : extendedPublicKeys.entrySet()) { diff --git a/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java b/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java index 5a9bf48..c44f44c 100644 --- a/src/main/java/com/sparrowwallet/drongo/psbt/PSBTInput.java +++ b/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; diff --git a/src/main/java/com/sparrowwallet/drongo/wallet/Wallet.java b/src/main/java/com/sparrowwallet/drongo/wallet/Wallet.java index 978c271..f12bc5a 100644 --- a/src/main/java/com/sparrowwallet/drongo/wallet/Wallet.java +++ b/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 utxoSelectors, Address recipientAddress, long recipientAmount, double feeRate, double longTermFeeRate, Long fee, boolean sendAll, boolean groupByAddress, boolean includeMempoolChange) throws InsufficientFundsException { + public WalletTransaction createWalletTransaction(List 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 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 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