Consider segwit when checking for standard max tx size

pom.xml

@@ -20,7 +20,7 @@
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <groupId>co.rsk.bitcoinj</groupId>
-    <version>0.14.4-rsk-18_2-SNAPSHOT</version>
+    <version>0.14.4-rsk-18</version>
     <artifactId>bitcoinj-thin</artifactId>
 
     <name>bitcoinj-thin</name>

src/main/java/co/rsk/bitcoinj/core/TransactionWitness.java

@@ -1,12 +1,14 @@
 package co.rsk.bitcoinj.core;
 
 import co.rsk.bitcoinj.crypto.TransactionSignature;
+import co.rsk.bitcoinj.script.RedeemScriptParser;
+import co.rsk.bitcoinj.script.RedeemScriptParserFactory;
+import co.rsk.bitcoinj.script.Script;
+import com.google.common.base.Preconditions;
 
 import javax.annotation.Nullable;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Objects;
+import java.util.*;
+import static com.google.common.base.Preconditions.checkState;
 
 public class TransactionWitness {
     static TransactionWitness empty = new TransactionWitness(0);
@@ -66,6 +68,105 @@ public byte[] getScriptBytes() {
             return pushes.get(pushes.size() - 1);
     }
 
+    /**
+     replicated logic from {@link Script#getSigInsertionIndex}
+     * */
+    public int getSigInsertionIndex(Sha256Hash sigHash, BtcECKey signingKey) {
+        int witnessSize = getPushCount();
+        int redeemScriptIndex = witnessSize - 1;
+        byte[] redeemScriptData = getPush(redeemScriptIndex);
+        Script redeemScript = new Script(redeemScriptData);
+        RedeemScriptParser redeemScriptParser = RedeemScriptParserFactory.get(redeemScript.getChunks());
+
+        int sigInsertionIndex = 0;
+        int keyIndexInRedeem = redeemScriptParser.findKeyInRedeem(signingKey);
+
+        byte[] emptyByte = new byte[]{};
+        // the pushes that should have the signatures
+        // are between first one (empty byte for checkmultisig bug)
+        // and second to last one (op_notif + redeem script)
+        for (int i = 1; i < getPushCount() - 1; i ++) {
+            byte[] push = getPush(i);
+            Preconditions.checkNotNull(push);
+            if (!Arrays.equals(push, emptyByte)) {
+                if (keyIndexInRedeem < redeemScriptParser.findSigInRedeem(push, sigHash)) {
+                    return sigInsertionIndex;
+                }
+
+                sigInsertionIndex++;
+            }
+        }
+
+        return sigInsertionIndex;
+    }
+
+    /**
+        replicated logic from {@link Script#getScriptSigWithSignature}
+    * */
+    public TransactionWitness updateWitnessWithSignature(Script outputScript, byte[] signature, int targetIndex) {
+        int sigsPrefixCount = outputScript.getSigsPrefixCount();
+        int sigsSuffixCount = outputScript.getSigsSuffixCount();
+        return updateWitnessWithSignature(signature, targetIndex, sigsPrefixCount, sigsSuffixCount);
+    }
+
+    /**
+        replicated logic from {@link co.rsk.bitcoinj.script.ScriptBuilder#updateScriptWithSignature}
+     * */
+    private TransactionWitness updateWitnessWithSignature(byte[] signature, int targetIndex, int sigsPrefixCount, int sigsSuffixCount) {
+        int totalPushes = getPushCount();
+
+        byte[] emptyByte = new byte[]{};
+        // since we fill the signatures in order, checking
+        // the second to last push is enough to know
+        // if there's space for new signatures
+        byte[] secondToLastPush = getPush(totalPushes - sigsSuffixCount - 1);
+        boolean hasMissingSigs = Arrays.equals(secondToLastPush, emptyByte);
+        Preconditions.checkArgument(hasMissingSigs, "Witness script is already filled with signatures");
+
+        List<byte[]> updatedPushes = new ArrayList<>();
+        // the signatures appear after the prefix
+        for (int i = 0; i < sigsPrefixCount; i++) {
+            byte[] push = getPush(i);
+            updatedPushes.add(push);
+        }
+
+        int index = 0;
+        boolean inserted = false;
+        // copy existing sigs
+        for (int i = sigsPrefixCount; i < totalPushes - sigsSuffixCount; i++) {
+            if (index == targetIndex) {
+                inserted = true;
+                updatedPushes.add(signature);
+                ++index;
+            }
+
+            byte[] push = getPush(i);
+            if (!Arrays.equals(push, emptyByte)) {
+                updatedPushes.add(push);
+                ++index;
+            }
+        }
+
+        // add zeros for missing signatures
+        while (index < totalPushes - sigsPrefixCount - sigsSuffixCount) {
+            if (index == targetIndex) {
+                inserted = true;
+                updatedPushes.add(signature);
+            } else {
+                updatedPushes.add(emptyByte);
+            }
+            index++;
+        }
+
+        // copy the suffix
+        for (int i = totalPushes - sigsSuffixCount; i < totalPushes; i++) {
+            byte[] push = getPush(i);
+            updatedPushes.add(push);
+        }
+
+        checkState(inserted);
+        return TransactionWitness.of(updatedPushes);
+    }
 
     @Override
     public boolean equals(Object otherObject) {

src/main/java/co/rsk/bitcoinj/script/RedeemScriptValidator.java

@@ -5,16 +5,18 @@
 
 public class RedeemScriptValidator {
 
+    private RedeemScriptValidator() {
+        // Prevent instantiation
+    }
+
     protected static boolean isRedeemLikeScript(List<ScriptChunk> chunks) {
         if (chunks.size() < 4) {
             return false;
         }
 
         ScriptChunk lastChunk = chunks.get(chunks.size() - 1);
         // A standard multisig redeem script must end in OP_CHECKMULTISIG[VERIFY]
-        boolean isStandard = lastChunk.isOpCode() &&
-                (lastChunk.equalsOpCode(ScriptOpCodes.OP_CHECKMULTISIG) ||
-                    lastChunk.equalsOpCode(ScriptOpCodes.OP_CHECKMULTISIGVERIFY));
+        boolean isStandard = lastChunk.isOpCheckMultiSig();
         if (isStandard) {
             return true;
         }
@@ -23,9 +25,7 @@ protected static boolean isRedeemLikeScript(List<ScriptChunk> chunks) {
         ScriptChunk penultimateChunk = chunks.get(chunks.size() - 2);
         return lastChunk.isOpCode() &&
             lastChunk.equalsOpCode(ScriptOpCodes.OP_ENDIF) &&
-            penultimateChunk.isOpCode() &&
-            (penultimateChunk.equalsOpCode(ScriptOpCodes.OP_CHECKMULTISIG) ||
-                penultimateChunk.equalsOpCode(ScriptOpCodes.OP_CHECKMULTISIGVERIFY));
+            penultimateChunk.isOpCheckMultiSig();
     }
 
     protected static boolean hasStandardRedeemScriptStructure(List<ScriptChunk> chunks) {
@@ -34,32 +34,38 @@ protected static boolean hasStandardRedeemScriptStructure(List<ScriptChunk> chun
                 return false;
             }
 
-            // First chunk must be an OP_N
-            if (!isOpN(chunks.get(0))) {
+            // last chunk should be OP_CHECKMULTISIG
+            int chunksSize = chunks.size();
+            ScriptChunk lastChunk = chunks.get(chunksSize - 1);
+            if (!lastChunk.isOpCheckMultiSig()) {
                 return false;
             }
 
-            // Second to last chunk must be an OP_N opcode too, and there should be
-            // that many data chunks (keys).
-            ScriptChunk secondToLastChunk = chunks.get(chunks.size() - 2);
-            if (!isOpN(secondToLastChunk)) {
+            // First chunk must be a number for the threshold
+            ScriptChunk firstChunk = chunks.get(0);
+            // Second to last chunk must be a number for the keys
+            int secondToLastChunkIndex = chunksSize - 2;
+            ScriptChunk secondToLastChunk = chunks.get(secondToLastChunkIndex);
+
+            if (!(firstChunk.isPositiveN() && secondToLastChunk.isPositiveN())) {
                 return false;
             }
 
-            int numKeys = Script.decodeFromOpN(secondToLastChunk.opcode);
-            if (numKeys < 1 || chunks.size() != numKeys + 3) { // numKeys + M + N + OP_CHECKMULTISIG
+            int numKeys = secondToLastChunk.decodePositiveN();
+            // and there should be numKeys+3 total chunks (keys + OP_M + OP_N + OP_CHECKMULTISIG)
+            if (chunksSize != numKeys + 3) {
                 return false;
             }
 
-            for (int i = 1; i < chunks.size() - 2; i++) {
+            for (int i = 1; i < secondToLastChunkIndex; i++) {
                 if (chunks.get(i).isOpCode()) { // Should be the public keys, not op_codes
                     return false;
                 }
             }
 
             return true;
         } catch (IllegalStateException e) {
-            return false;   // Not an OP_N opcode.
+            return false; // Not a number
         }
     }
 
@@ -68,10 +74,11 @@ protected static boolean hasP2shErpRedeemScriptStructure(List<ScriptChunk> chunk
             return false;
         }
 
-        ScriptChunk firstChunk = chunks.get(0);
+        int opNotifIndex = 0;
+        boolean hasErpPrefix = chunks.get(opNotifIndex).equalsOpCode(ScriptOpCodes.OP_NOTIF);
 
-        boolean hasErpPrefix = firstChunk.opcode == ScriptOpCodes.OP_NOTIF;
-        boolean hasEndIfOpcode = chunks.get(chunks.size() - 1).equalsOpCode(ScriptOpCodes.OP_ENDIF);
+        int lastChunkIndex = chunks.size() - 1;
+        boolean hasEndIfOpcode = chunks.get(lastChunkIndex).equalsOpCode(ScriptOpCodes.OP_ENDIF);
 
         if (!hasErpPrefix || !hasEndIfOpcode) {
             return false;
@@ -101,8 +108,7 @@ protected static boolean hasP2shErpRedeemScriptStructure(List<ScriptChunk> chunk
             return false;
         }
 
-        /***
-         * Expected structure:
+        /* The redeem script structure should be as follows:
          * OP_NOTIF
          *  OP_M
          *  PUBKEYS...N
@@ -119,7 +125,6 @@ protected static boolean hasP2shErpRedeemScriptStructure(List<ScriptChunk> chunk
          *  OP_CHECKMULTISIG
          * OP_ENDIF
          */
-
         // Validate both default and erp federations redeem scripts.
         // Extract the default PowPeg and the emergency multisig redeemscript chunks
         List<ScriptChunk> defaultFedRedeemScriptChunks = chunks.subList(1, elseOpcodeIndex);
@@ -218,9 +223,4 @@ protected static List<ScriptChunk> removeOpCheckMultisig(Script redeemScript) {
         // Remove the last chunk, which has CHECKMULTISIG op code
         return redeemScript.getChunks().subList(0, redeemScript.getChunks().size() - 1);
     }
-
-    protected static boolean isOpN(ScriptChunk chunk) {
-        return chunk.isOpCode() &&
-            chunk.opcode >= ScriptOpCodes.OP_1 && chunk.opcode <= ScriptOpCodes.OP_16;
-    }
 }

src/main/java/co/rsk/bitcoinj/script/Script.java

@@ -22,15 +22,7 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import co.rsk.bitcoinj.core.Address;
-import co.rsk.bitcoinj.core.BtcECKey;
-import co.rsk.bitcoinj.core.BtcTransaction;
-import co.rsk.bitcoinj.core.NetworkParameters;
-import co.rsk.bitcoinj.core.ProtocolException;
-import co.rsk.bitcoinj.core.ScriptException;
-import co.rsk.bitcoinj.core.Sha256Hash;
-import co.rsk.bitcoinj.core.UnsafeByteArrayOutputStream;
-import co.rsk.bitcoinj.core.Utils;
+import co.rsk.bitcoinj.core.*;
 import co.rsk.bitcoinj.crypto.TransactionSignature;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
@@ -40,14 +32,7 @@
 import java.math.BigInteger;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -97,6 +82,9 @@ public enum VerifyFlag {
 
     private static final Logger log = LoggerFactory.getLogger(Script.class);
     public static final long MAX_SCRIPT_ELEMENT_SIZE = 520;  // bytes
+
+    /** The maximum size in bytes of a standard witnessScript */
+    public static final long MAX_STANDARD_P2WSH_SCRIPT_SIZE = 3600;
     public static final int SIG_SIZE = 75;
     /** Max number of sigops allowed in a standard p2sh redeem script */
     public static final int MAX_P2SH_SIGOPS = 15;
@@ -459,19 +447,25 @@ private boolean isErpType(Script redeemScript) {
      * Returns a copy of the given scriptSig with the signature inserted in the given position.
      */
     public Script getScriptSigWithSignature(Script scriptSig, byte[] sigBytes, int index) {
-        int sigsPrefixCount = 0;
-        int sigsSuffixCount = 0;
-        if (isPayToScriptHash()) {
-            sigsPrefixCount = 1; // OP_0 <sig>* <redeemScript>
-            sigsSuffixCount = 1;
-        } else if (isSentToMultiSig()) {
-            sigsPrefixCount = 1; // OP_0 <sig>*
-        } else if (isSentToAddress()) {
-            sigsSuffixCount = 1; // <sig> <pubkey>
-        }
+        int sigsPrefixCount = getSigsPrefixCount();
+        int sigsSuffixCount = getSigsSuffixCount();
         return ScriptBuilder.updateScriptWithSignature(scriptSig, sigBytes, index, sigsPrefixCount, sigsSuffixCount);
     }
 
+    public int getSigsPrefixCount() {
+        if (isPayToScriptHash() || isSentToMultiSig()) { // OP_0 <sig>* || OP_0 <sig>*
+            return 1;
+        }
+        return 0;
+    }
+
+    public int getSigsSuffixCount() {
+        if (isPayToScriptHash() || isSentToAddress()) {  // <sig>* <redeemScript> || <sig> <pubkey>
+            return 1;
+        }
+        return 0;
+    }
+
     private RedeemScriptParser getRedeemScriptParser() {
         if (redeemScriptParser == null){
             redeemScriptParser = RedeemScriptParserFactory.get(chunks);
@@ -554,22 +548,24 @@ private static int getSigOpCount(List<ScriptChunk> chunks, boolean accurate) thr
 
     static int decodeFromOpN(int opcode) {
         checkArgument((opcode == OP_0 || opcode == OP_1NEGATE) || (opcode >= OP_1 && opcode <= OP_16), "decodeFromOpN called on non OP_N opcode");
-        if (opcode == OP_0)
+        if (opcode == OP_0) {
             return 0;
-        else if (opcode == OP_1NEGATE)
+        } else if (opcode == OP_1NEGATE) {
             return -1;
-        else
+        } else {
             return opcode + 1 - OP_1;
+        }
     }
 
     static int encodeToOpN(int value) {
         checkArgument(value >= -1 && value <= 16, "encodeToOpN called for " + value + " which we cannot encode in an opcode.");
-        if (value == 0)
+        if (value == 0) {
             return OP_0;
-        else if (value == -1)
+        } else if (value == -1) {
             return OP_1NEGATE;
-        else
+        } else {
             return value - 1 + OP_1;
+        }
     }
 
     /**