Merge pull request #20 from twostack/signature-bugfix

Second Signature Bugfix

Author: stephanfeb

CHANGELOG.md

@@ -1,3 +1,10 @@
+#Release 1.6.4
+### Second BugFix for Signature Generation
+
+- When spending multiple outputs from the same transaction it was possible
+  for some of the spending inputs to not get signed.
+  This update fixes this bug. 
+
 #Release 1.6.3
 ### BugFix for Signature Generation
 

build.gradle

@@ -6,7 +6,7 @@ plugins {
 }
 
 group 'org.twostack'
-version '1.6.3'
+version '1.6.4'
 
 repositories {
     mavenCentral()

src/main/java/org/twostack/bitcoin4j/transaction/TransactionBuilder.java

@@ -116,7 +116,7 @@ public TransactionBuilder spendFromUtxoMap(TransactionSigner signer, Map<String,
         outpoint.setSatoshis((BigInteger)utxoMap.get("satoshis"));
         outpoint.setTransactionId(transactionId);
 
-        this.signerMap.put(transactionId, new SignerDto(signer, outpoint));
+        this.signerMap.put(transactionId + ":" + outputIndex, new SignerDto(signer, outpoint));
 
         if (unlocker == null){
             unlocker = new DefaultUnlockBuilder();
@@ -186,7 +186,7 @@ public TransactionBuilder spendFromTransaction(TransactionSigner signer, Transac
         outpoint.setSatoshis(output.getAmount());
         outpoint.setTransactionId(transactionId);
 
-        this.signerMap.put(transactionId, new SignerDto(signer, outpoint));
+        this.signerMap.put(transactionId + ":" + outputIndex, new SignerDto(signer, outpoint));
 
         //update the spending transactionInput
         TransactionInput input = new TransactionInput(
@@ -220,7 +220,7 @@ public TransactionBuilder spendFromTransaction(Transaction txn, int outputIndex,
 
     public TransactionBuilder spendFromOutpoint(TransactionSigner signer, TransactionOutpoint outpoint, long sequenceNumber, UnlockingScriptBuilder unlocker) {
 
-        this.signerMap.put(outpoint.getTransactionId(), new SignerDto(signer, outpoint));
+        this.signerMap.put(outpoint.getTransactionId() +  ":" + outpoint.getOutputIndex(), new SignerDto(signer, outpoint));
 
         TransactionInput input = new TransactionInput(
                 HEX.decode(outpoint.getTransactionId()),
@@ -275,7 +275,7 @@ public TransactionBuilder spendFromOutput(TransactionSigner signer, String utxoT
         outpoint.setSatoshis(amount);
         outpoint.setTransactionId(utxoTxnId);
 
-        this.signerMap.put(utxoTxnId, new SignerDto(signer, outpoint));
+        this.signerMap.put(utxoTxnId +  ":" + outputIndex, new SignerDto(signer, outpoint));
 
         TransactionInput input = new TransactionInput(
                 HEX.decode(utxoTxnId),
@@ -434,9 +434,9 @@ public Transaction build(boolean performChecks) throws TransactionException, IOE
             TransactionInput currentInput = inputs.get(index);
 
             List<Map.Entry<String, SignerDto>> result = signerMap.entrySet().stream().filter( (Map.Entry<String, SignerDto> entry) -> {
-                //drop everything from stream except what we're looking for
-                return (entry.getValue().outpoint.getTransactionId().equals(HEX.encode(currentInput.getPrevTxnId())) &&
-                        entry.getValue().outpoint.getOutputIndex() == currentInput.getPrevTxnOutputIndex());
+                String entryKey = entry.getValue().outpoint.getTransactionId() + ":" + entry.getValue().outpoint.getOutputIndex();
+                String currentInputKey = Utils.HEX.encode(currentInput.getPrevTxnId()) + ":" + currentInput.getPrevTxnOutputIndex();
+                return entryKey.equals(currentInputKey);
             }).collect(Collectors.toList());
 
             if (result.size() > 0) {

src/test/java/org/twostack/bitcoin4j/transaction/TransactionBuilderTest.java

@@ -8,22 +8,20 @@
 import org.twostack.bitcoin4j.Coin;
 import org.twostack.bitcoin4j.PrivateKey;
 import org.twostack.bitcoin4j.Utils;
-import org.twostack.bitcoin4j.exception.InvalidKeyException;
-import org.twostack.bitcoin4j.exception.SigHashException;
-import org.twostack.bitcoin4j.exception.SignatureDecodeException;
-import org.twostack.bitcoin4j.exception.TransactionException;
+import org.twostack.bitcoin4j.crypto.*;
+import org.twostack.bitcoin4j.exception.*;
 import org.twostack.bitcoin4j.params.NetworkAddressType;
+import org.twostack.bitcoin4j.params.NetworkType;
 import org.twostack.bitcoin4j.script.Interpreter;
 import org.twostack.bitcoin4j.script.Script;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.*;
+
+import static org.twostack.bitcoin4j.Utils.WHITESPACE_SPLITTER;
 
 public class TransactionBuilderTest {
 
@@ -270,7 +268,7 @@ public void builderCanSpendFromUtxoMap() throws InvalidKeyException, IOException
             UnlockingScriptBuilder unlocker = new P2PKHUnlockBuilder(privateKey.getPublicKey());
 
             TransactionSigner signer = new TransactionSigner(SigHashType.ALL.value | SigHashType.FORKID.value, privateKey);
-            builder.spendFromUtxoMap(signer, utxoMap,  unlocker);
+            builder.spendFromUtxoMap(signer, utxoMap, unlocker);
 
         }
 
@@ -308,4 +306,70 @@ public void builderCanSpendFromUtxoMap() throws InvalidKeyException, IOException
             }
         }).doesNotThrowAnyException();
     }
+
+    @Test
+    public void canSpendMultipleOutputsFromSameTx() throws MnemonicException, IOException, InvalidKeyException, TransactionException, SigHashException, SignatureDecodeException {
+
+        PrivateKey pkOne = PrivateKey.fromWIF("L14C38sujYefjpj4Zj5HTwjaHAZKMSLZZAnzLNojC8CmU8Q4TpWE");
+        PrivateKey pkTwo = PrivateKey.fromWIF("L4YDdzJa5Ae2ukENpPqFBPCutM1xsM9WvKFy6Bugv4iyg6U4c2kE");
+
+        TransactionBuilder builder = new TransactionBuilder();
+        builder.withFeePerKb(512);
+
+        String rawFundingTx = "0100000001c5d4b2f482627e7c46ad977cbacf9bfdf2197229952daa3a4b57d15fccfad92b000000006a47304402207927136ea0b51fc9a2cb883ac2a72410dec41bef98062b7845eac691cc2c9f6602202b8b370b588542941623379a046e5aea654b1edd5c9c074a426b1be45545ed5d412103c36ea9ccb9a332b415ebf9e9823e2b352f2ed2c4199ea382cf98ddbfcf4eed24ffffffff02204e0000000000001976a914eb6edc362ae7e5d2765e86a97741722b0f7e20d688ac260c0300000000001976a914cd1a22818d1f143b152276ee6a69486233405d3e88ac00000000";
+        Transaction fundingTx = Transaction.fromHex(rawFundingTx);
+
+        P2PKHUnlockBuilder unlockOne = new P2PKHUnlockBuilder(pkOne.getPublicKey());
+        TransactionSigner signerOne = new TransactionSigner(SigHashType.ALL.value | SigHashType.FORKID.value, pkOne);
+        builder.spendFromTransaction(signerOne, fundingTx, 0, TransactionInput.MAX_SEQ_NUMBER, unlockOne);
+
+        P2PKHUnlockBuilder unlockTwo = new P2PKHUnlockBuilder(pkTwo.getPublicKey());
+        TransactionSigner signerTwo = new TransactionSigner(SigHashType.ALL.value | SigHashType.FORKID.value, pkTwo);
+        builder.spendFromTransaction(signerTwo, fundingTx, 1, TransactionInput.MAX_SEQ_NUMBER, unlockTwo);
+
+        //send back to the first key
+        Address recipientAddress = Address.fromKey(NetworkAddressType.MAIN_PKH, pkOne.getPublicKey());
+        P2PKHLockBuilder lockBuilder = new P2PKHLockBuilder(recipientAddress);
+        builder.spendTo(lockBuilder, BigInteger.valueOf(20000));
+
+        //send change to second key
+        Address changeAddress = Address.fromKey(NetworkAddressType.MAIN_PKH, pkTwo.getPublicKey());
+        builder.sendChangeTo(changeAddress);
+
+        Transaction broadcastTx = builder.build(true);
+
+        Assertions.assertThatCode(() -> {
+
+            //new Script Interpreter to help us verify our spending conditions
+            Interpreter interp = new Interpreter();
+            Set<Script.VerifyFlag> verifyFlags = new HashSet<Script.VerifyFlag>(Arrays.asList(Script.VerifyFlag.SIGHASH_FORKID));
+
+
+//            ////FIRST UTXO
+            Integer fundingOutputIndexOne = 0;
+            Long fundingValueOne = 20000L;
+
+            //lookup funding transaction corresponding to first output
+            TransactionOutput fundingOutput = fundingTx.getOutputs().get(fundingOutputIndexOne);
+
+            //assert that funding transaction is spending correctly
+            interp.correctlySpends(broadcastTx.getInputs().get(0).getScriptSig(), fundingOutput.getScript(), broadcastTx, 0, verifyFlags, Coin.valueOf(fundingValueOne));
+
+            ////SECOND UTXO
+//            TransactionInput spendingInputTwo = TransactionInput.fromByteArray(broadcastTx.getInputs().get(1).serialize());
+            Integer fundingOutputIndexTwo = 1;
+            Long fundingValueTwo = 199718L;
+
+            //lookup funding transaction corresponding to first output
+            TransactionOutput fundingOutputTwo = fundingTx.getOutputs().get(fundingOutputIndexTwo);
+
+            Transaction signedTxTwo = signerTwo.sign(broadcastTx, fundingOutputTwo, 1);
+
+            //assert that funding transaction is spending correctly
+            interp.correctlySpends(broadcastTx.getInputs().get(1).getScriptSig(), fundingOutputTwo.getScript(), broadcastTx, 1, verifyFlags, Coin.valueOf(fundingValueTwo));
+
+        }).doesNotThrowAnyException();
+
+    }
+
 }