Merge pull request #19 from twostack/txbuilder-api

stephanfeb · web-flow · commit 8c5953290202 · 2022-01-27T14:43:21.000+08:00
BugFix for Signature Generation
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,11 @@
+#Release 1.6.3
+### BugFix for Signature Generation
+
+- Bug was introduced by previous feature where spending and signing 
+  multiple inputs resulted in invalid signatures being created. Fixed.
+- Added additional testing via the Script Interpreter to verify that 
+  signatures don't break and that utxos are spent correctly. 
+
 #Release 1.6.2
 ### Foot-in-mouth release
 
diff --git a/build.gradle b/build.gradle
@@ -6,7 +6,7 @@ plugins {
 }
 
 group 'org.twostack'
-version '1.6.2'
+version '1.6.3'
 
 repositories {
     mavenCentral()
diff --git a/src/main/java/org/twostack/bitcoin4j/transaction/TransactionBuilder.java b/src/main/java/org/twostack/bitcoin4j/transaction/TransactionBuilder.java
@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.math.BigInteger;
 import java.util.*;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static org.twostack.bitcoin4j.Utils.HEX;
@@ -421,20 +422,26 @@ public Transaction build(boolean performChecks) throws TransactionException, IOE
         //add transaction outputs
         tx.addOutputs(outputs);
 
+        if (changeScriptBuilder != null) {
+            tx.addOutput(getChangeOutput());
+        }
+
+        tx.setLockTime(nLockTime);
+
         //update inputs with signatures
-        String txId = tx.getTransactionId();
+//        String txId = tx.getTransactionId();
         for (int index = 0; index < inputs.size() ; index++) {
             TransactionInput currentInput = inputs.get(index);
 
-            Optional<Map.Entry<String, SignerDto>> result = signerMap.entrySet().stream().filter( (Map.Entry<String, SignerDto> entry) -> {
+            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() == HEX.encode(currentInput.getPrevTxnId()) &&
+                return (entry.getValue().outpoint.getTransactionId().equals(HEX.encode(currentInput.getPrevTxnId())) &&
                         entry.getValue().outpoint.getOutputIndex() == currentInput.getPrevTxnOutputIndex());
-            }).findFirst();
+            }).collect(Collectors.toList());
 
-            if (result.isPresent()) {
+            if (result.size() > 0) {
 
-                SignerDto dto = result.get().getValue();
+                SignerDto dto = result.get(0).getValue();
                 TransactionOutput utxoToSpend = new TransactionOutput(dto.outpoint.getSatoshis(), dto.outpoint.getLockingScript());
 
                 //TODO: this side-effect programming where the signer mutates my local variable
@@ -443,14 +450,6 @@ public Transaction build(boolean performChecks) throws TransactionException, IOE
             }
         }
 
-
-
-        if (changeScriptBuilder != null) {
-            tx.addOutput(getChangeOutput());
-        }
-
-        tx.setLockTime(nLockTime);
-
         return tx;
 
     }
diff --git a/src/test/java/org/twostack/bitcoin4j/transaction/TransactionBuilderTest.java b/src/test/java/org/twostack/bitcoin4j/transaction/TransactionBuilderTest.java
@@ -5,19 +5,25 @@
 import org.assertj.core.api.Assertions;
 import org.junit.Test;
 import org.twostack.bitcoin4j.Address;
+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.params.NetworkAddressType;
+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;
 
 public class TransactionBuilderTest {
 
@@ -55,10 +61,35 @@ public void processAndSignMultiInput() throws IOException, InvalidKeyException,
 
 
         Assertions.assertThatCode(() -> {
-            Transaction broadcastTx = builder.withFeePerKb(512)
+            Transaction broadcastTx = builder.withFeePerKb(1024)
                     .spendTo(new P2PKHLockBuilder(recipientAddress), BigInteger.valueOf(100000))
                     .sendChangeTo(recipientAddress)
                     .build(true);
+
+            //new Script Interpreter to help us verify our spending conditions
+            Interpreter interp = new Interpreter();
+
+            //loop over every one of our spending inputs and verify we are
+            //correctly spending those outputs
+            for (int scriptSigIndex = 0; scriptSigIndex < 0; scriptSigIndex++) {
+                TransactionInput spendingInputOne = TransactionInput.fromByteArray(broadcastTx.getInputs().get(0).serialize());
+                String fundingTxId = Utils.HEX.encode(spendingInputOne.getPrevTxnId());
+                Integer fundingOutputIndex = json.get(scriptSigIndex).get("tx_pos").asInt();
+                Long fundingValue = json.get(scriptSigIndex).get("value").asLong();
+
+                //lookup funding transaction corresponding to first output
+                String rawFundingTx = json.get(scriptSigIndex).get("raw_tx").asText();
+                Transaction fundingTxOne = Transaction.fromHex(rawFundingTx);
+                TransactionOutput fundingOutput = fundingTxOne.getOutputs().get(fundingOutputIndex);
+
+                TransactionSigner signer = new TransactionSigner(SigHashType.FORKID.value | SigHashType.ALL.value, privateKey);
+                Transaction signedTx = signer.sign(broadcastTx, fundingOutput, scriptSigIndex);
+
+                //assert that funding transaction is spending correctly
+                Set<Script.VerifyFlag> verifyFlags = new HashSet<Script.VerifyFlag>(Arrays.asList(Script.VerifyFlag.SIGHASH_FORKID));
+                interp.correctlySpends(signedTx.getInputs().get(scriptSigIndex).getScriptSig(), fundingOutput.getScript(), broadcastTx, scriptSigIndex, verifyFlags, Coin.valueOf(fundingValue));
+            }
+
         }).doesNotThrowAnyException();
 
     }
@@ -101,6 +132,30 @@ public void builderCanSpendFromOutput() throws InvalidKeyException, IOException
                     .spendTo(new P2PKHLockBuilder(recipientAddress), BigInteger.valueOf(100000))
                     .sendChangeTo(recipientAddress)
                     .build(true);
+
+            //new Script Interpreter to help us verify our spending conditions
+            Interpreter interp = new Interpreter();
+
+            //loop over every one of our spending inputs and verify we are
+            //correctly spending those outputs
+            for (int scriptSigIndex = 0; scriptSigIndex < 0; scriptSigIndex++) {
+                TransactionInput spendingInputOne = TransactionInput.fromByteArray(broadcastTx.getInputs().get(0).serialize());
+                String fundingTxId = Utils.HEX.encode(spendingInputOne.getPrevTxnId());
+                Integer fundingOutputIndex = json.get(scriptSigIndex).get("tx_pos").asInt();
+                Long fundingValue = json.get(scriptSigIndex).get("value").asLong();
+
+                //lookup funding transaction corresponding to first output
+                String rawFundingTx = json.get(scriptSigIndex).get("raw_tx").asText();
+                Transaction fundingTxOne = Transaction.fromHex(rawFundingTx);
+                TransactionOutput fundingOutput = fundingTxOne.getOutputs().get(fundingOutputIndex);
+
+                TransactionSigner signer = new TransactionSigner(SigHashType.FORKID.value | SigHashType.ALL.value, privateKey);
+                Transaction signedTx = signer.sign(broadcastTx, fundingOutput, scriptSigIndex);
+
+                //assert that funding transaction is spending correctly
+                Set<Script.VerifyFlag> verifyFlags = new HashSet<Script.VerifyFlag>(Arrays.asList(Script.VerifyFlag.SIGHASH_FORKID));
+                interp.correctlySpends(signedTx.getInputs().get(scriptSigIndex).getScriptSig(), fundingOutput.getScript(), broadcastTx, scriptSigIndex, verifyFlags, Coin.valueOf(fundingValue));
+            }
         }).doesNotThrowAnyException();
 
     }
@@ -152,6 +207,30 @@ public void builderCanSpendFromOutpoint() throws InvalidKeyException, IOExceptio
                     .spendTo(new P2PKHLockBuilder(recipientAddress), BigInteger.valueOf(100000))
                     .sendChangeTo(recipientAddress)
                     .build(true);
+
+            //new Script Interpreter to help us verify our spending conditions
+            Interpreter interp = new Interpreter();
+
+            //loop over every one of our spending inputs and verify we are
+            //correctly spending those outputs
+            for (int scriptSigIndex = 0; scriptSigIndex < 0; scriptSigIndex++) {
+                TransactionInput spendingInputOne = TransactionInput.fromByteArray(broadcastTx.getInputs().get(0).serialize());
+                String fundingTxId = Utils.HEX.encode(spendingInputOne.getPrevTxnId());
+                Integer fundingOutputIndex = json.get(scriptSigIndex).get("tx_pos").asInt();
+                Long fundingValue = json.get(scriptSigIndex).get("value").asLong();
+
+                //lookup funding transaction corresponding to first output
+                String rawFundingTx = json.get(scriptSigIndex).get("raw_tx").asText();
+                Transaction fundingTxOne = Transaction.fromHex(rawFundingTx);
+                TransactionOutput fundingOutput = fundingTxOne.getOutputs().get(fundingOutputIndex);
+
+                TransactionSigner signer = new TransactionSigner(SigHashType.FORKID.value | SigHashType.ALL.value, privateKey);
+                Transaction signedTx = signer.sign(broadcastTx, fundingOutput, scriptSigIndex);
+
+                //assert that funding transaction is spending correctly
+                Set<Script.VerifyFlag> verifyFlags = new HashSet<Script.VerifyFlag>(Arrays.asList(Script.VerifyFlag.SIGHASH_FORKID));
+                interp.correctlySpends(signedTx.getInputs().get(scriptSigIndex).getScriptSig(), fundingOutput.getScript(), broadcastTx, scriptSigIndex, verifyFlags, Coin.valueOf(fundingValue));
+            }
         }).doesNotThrowAnyException();
     }
 
@@ -165,6 +244,7 @@ public void builderCanSpendFromUtxoMap() throws InvalidKeyException, IOException
         String wif = "cRTUuWgPdp7tJPrn1Xeq196eZa4ZCpg8n3cgDJsJmgDHBZ8x9fpv";
         PrivateKey privateKey = PrivateKey.fromWIF(wif);
 
+        System.out.println(Address.fromKey(NetworkAddressType.TEST_PKH, privateKey.getPublicKey()).toString());
         JsonNode json = new ObjectMapper().readTree(
                 new InputStreamReader(getClass().getResourceAsStream("multi_input.json"),
                         StandardCharsets.UTF_8)
@@ -202,6 +282,30 @@ public void builderCanSpendFromUtxoMap() throws InvalidKeyException, IOException
                     .spendTo(new P2PKHLockBuilder(recipientAddress), BigInteger.valueOf(100000))
                     .sendChangeTo(recipientAddress)
                     .build(true);
+
+            //new Script Interpreter to help us verify our spending conditions
+            Interpreter interp = new Interpreter();
+
+            //loop over every one of our spending inputs and verify we are
+            //correctly spending those outputs
+            for (int scriptSigIndex = 0; scriptSigIndex < 0; scriptSigIndex++) {
+                TransactionInput spendingInputOne = TransactionInput.fromByteArray(broadcastTx.getInputs().get(0).serialize());
+                String fundingTxId = Utils.HEX.encode(spendingInputOne.getPrevTxnId());
+                Integer fundingOutputIndex = json.get(scriptSigIndex).get("tx_pos").asInt();
+                Long fundingValue = json.get(scriptSigIndex).get("value").asLong();
+
+                //lookup funding transaction corresponding to first output
+                String rawFundingTx = json.get(scriptSigIndex).get("raw_tx").asText();
+                Transaction fundingTxOne = Transaction.fromHex(rawFundingTx);
+                TransactionOutput fundingOutput = fundingTxOne.getOutputs().get(fundingOutputIndex);
+
+                TransactionSigner signer = new TransactionSigner(SigHashType.FORKID.value | SigHashType.ALL.value, privateKey);
+                Transaction signedTx = signer.sign(broadcastTx, fundingOutput, scriptSigIndex);
+
+                //assert that funding transaction is spending correctly
+                Set<Script.VerifyFlag> verifyFlags = new HashSet<Script.VerifyFlag>(Arrays.asList(Script.VerifyFlag.SIGHASH_FORKID));
+                interp.correctlySpends(signedTx.getInputs().get(scriptSigIndex).getScriptSig(), fundingOutput.getScript(), broadcastTx, scriptSigIndex, verifyFlags, Coin.valueOf(fundingValue));
+            }
         }).doesNotThrowAnyException();
     }
 }
diff --git a/src/test/resources/org/twostack/bitcoin4j/transaction/multi_input_local.json b/src/test/resources/org/twostack/bitcoin4j/transaction/multi_input_local.json
@@ -0,0 +1,23 @@
+[
+  {
+    "height": 1433834,
+    "value": 500000000,
+    "tx_pos": 0,
+    "raw_tx": "0200000001f4145c67f9678e55584c9e258b3de751f74d1ceebd0cdfa7391fef8945e650a300000000484730440220717b3cc7930b5b7114e92dc239c27aa69314898aae642be43a731b696cddd0100220043d63d6b0f7b16cca5bcaed722ad80ec36480c112c7c6c34e4fe2b4f0837ba841feffffff020065cd1d000000001976a914191f99293aedd9f66e090e296abe2c64cabdbc5788ac80587307000000001976a914cad75dd4a8ea76c18ad5e561afd013daa4a8d0ec88ac6a020000",
+    "tx_hash": "9dc51e14a93ef538affe675200aeec15aedc0378c093074cf302f4d731eb484f"
+  },
+  {
+    "height": 1433834,
+    "value": 700000000,
+    "tx_pos": 0,
+    "tx_hash": "d2cbb85329f1f14f94e028edb8d2bc21cb3ffe9aed8b58f8741571580a186a38",
+    "raw_tx": "0200000001ddbb28b78fc3f6501afff0bbff1add8aea9301d906a706362b9ac9650418f6050000000047463043021f1f31214b5fa7deb37a72bfb4de421048e338669eec554d5dd7edbe3af337e302200e58d8a7964d0d26319950fa3ed3599dc9e338fea83577c207e9f788c77b604341feffffff020027b929000000001976a914191f99293aedd9f66e090e296abe2c64cabdbc5788acc054c820000000001976a91468bda6c59f5211caaefd49d69e45c56d88b4cd3788ac6a020000"
+  },
+  {
+    "height": 1433834,
+    "value": 600000000,
+    "tx_pos": 1,
+    "raw_tx": "02000000011a44b1703870432cb26a89a269b1bd546b3ff5fdc7775d5dda07154c897c878a000000004847304402205ebe801f4a5a693a607eac50d217653271295df3421b3c2c502739da04ecd2e1022019c018c1cd8f8692872b8f7e8ab61bc912b32e22f57ab680ccd7d4d8a4c97ad941feffffff0280777d01000000001976a914a99d5811b500ba6dec61dc4bf95fa96f4d610e6688ac0046c323000000001976a914191f99293aedd9f66e090e296abe2c64cabdbc5788ac6a020000",
+    "tx_hash": "67e7555620688f285b76f7cb4698679ffbd006029bd9c51d36d153b8bdbeb377"
+  }
+]

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ plugins {`
`6`	`6`	`}`
`7`	`7`
`8`	`8`	`group 'org.twostack'`
`9`		`-version '1.6.2'`
	`9`	`+version '1.6.3'`
`10`	`10`
`11`	`11`	`repositories {`
`12`	`12`	`mavenCentral()`