Update script_tests.json, fix bugs that were unveiled with that updat… (#799)

* Update script_tests.json, fix bugs that were unveiled with that update. Specifically around handling negative zero in Script, and cleanstack behavior for segwit scripts. We were not checking for equality at the byte level for negative zero. With cleanstack, we were failing the script, but not failing with the correct error. If you look at interpreter.cpp in bitcoin core, cleanstack check is done before checking if the stacktop is true or false

* Address code review

core-test/src/test/scala/org/bitcoins/core/script/interpreter/ScriptInterpreterTest.scala

@@ -1,21 +1,15 @@
 package org.bitcoins.core.script.interpreter
 
-import org.bitcoins.core.crypto.{
-  BaseTxSigComponent,
-  WitnessTxSigComponentP2SH,
-  WitnessTxSigComponentRaw
-}
+import org.bitcoins.core.crypto.{BaseTxSigComponent, WitnessTxSigComponentP2SH, WitnessTxSigComponentRaw}
 import org.bitcoins.core.currency.CurrencyUnits
 import org.bitcoins.core.protocol.script._
-import org.bitcoins.core.protocol.transaction.{
-  TransactionOutput,
-  WitnessTransaction
-}
+import org.bitcoins.core.protocol.transaction.{TransactionOutput, WitnessTransaction}
 import org.bitcoins.core.script.PreExecutionScriptProgram
 import org.bitcoins.core.script.flag.ScriptFlagFactory
 import org.bitcoins.core.script.interpreter.testprotocol.CoreTestCase
 import org.bitcoins.core.script.interpreter.testprotocol.CoreTestCaseProtocol._
 import org.bitcoins.core.util._
+import org.bitcoins.testkit.util.BitcoinSUnitTest
 import org.scalatest.{FlatSpec, MustMatchers}
 import org.slf4j.LoggerFactory
 import spray.json._
@@ -26,27 +20,16 @@ import scala.util.Try
 /**
   * Created by chris on 1/6/16.
   */
-class ScriptInterpreterTest extends FlatSpec with MustMatchers {
-  private val logger = LoggerFactory.getLogger(this.getClass)
+class ScriptInterpreterTest extends BitcoinSUnitTest {
+
   "ScriptInterpreter" must "evaluate all the scripts from the bitcoin core script_tests.json" in {
 
     val source = Source.fromURL(getClass.getResource("/script_tests.json"))
 
     //use this to represent a single test case from script_valid.json
-    /*    val lines =
+/*        val lines =
       """
-          | [[
-          |    [
-          |        "304402200929d11561cd958460371200f82e9cae64c727a495715a31828e27a7ad57b36d0220361732ced04a6f97351ecca21a56d0b8cd4932c1da1f8f569a2b68e5e48aed7801",
-          |        "0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8",
-          |        0.00000001
-          |    ],
-          |    "0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
-          |    "HASH160 0x14 0x17743beb429c55c942d2ec703b98c4d57c2df5c6 EQUAL",
-          |    "P2SH,WITNESS",
-          |    "OK",
-          |    "Basic P2SH(P2WPKH)"
-          |]]
+          | [["0x01 0x80", "DUP BOOLOR", "P2SH,STRICTENC", "EVAL_FALSE", "negative-0 negative-0 BOOLOR"]]
    """.stripMargin*/
     val lines =
       try source.getLines.filterNot(_.isEmpty).map(_.trim) mkString "\n"

core/src/main/scala/org/bitcoins/core/script/arithmetic/ArithmeticInterpreter.scala

@@ -98,11 +98,12 @@ sealed abstract class ArithmeticInterpreter {
       program: ExecutionInProgressScriptProgram): StartedScriptProgram = {
     require(program.script.headOption.contains(OP_BOOLAND),
             "Script top must be OP_BOOLAND")
-    performBinaryBooleanOperation(program, (x, y) => {
-      val xIsFalse = (x == ScriptNumber.zero || x == OP_0)
-      val yIsFalse = (y == ScriptNumber.zero || y == OP_0)
-      if (xIsFalse || yIsFalse) false else true
-    })
+    performBinaryBooleanOperation(
+      program,
+      (x, y) => {
+        !ScriptNumberUtil.isZero(x) && !ScriptNumberUtil.isZero(y)
+      }
+    )
   }
 
   /** If a or b is not 0, the output is 1. Otherwise 0. */
@@ -111,7 +112,7 @@ sealed abstract class ArithmeticInterpreter {
     require(program.script.headOption.contains(OP_BOOLOR),
             "Script top must be OP_BOOLOR")
     performBinaryBooleanOperation(program, (x, y) => {
-      if (x == y && (x == ScriptNumber.zero || x == OP_0)) false else true
+      !ScriptNumberUtil.isZero(x) || !ScriptNumberUtil.isZero(y)
     })
   }
 

core/src/main/scala/org/bitcoins/core/script/constant/ScriptNumberUtil.scala

@@ -161,6 +161,16 @@ trait ScriptNumberUtil {
   def firstByteAllZeros(hex: String): Boolean =
     firstByteAllZeros(BitcoinSUtil.decodeHex(hex))
 
+  /** Checks if the two given [[ScriptNumber numbers]] are equivalent to zero
+    * in Script. Unfortunatey Script is one's complement which means we have
+    * things like negative zero, and also there isn't an enforcement of a
+    * minimal representation of zero, which means 0x00 = 0x0000 = 0x0000000.. == OP_0*/
+  def isZero(x: ScriptNumber): Boolean = {
+    val xIsFalse = x == ScriptNumber.zero || x == OP_0
+    val isNegZero = x == ScriptNumber.negativeZero
+    val isZero = x.toLong == 0
+    xIsFalse || isNegZero || isZero
+  }
 }
 
 object ScriptNumberUtil extends ScriptNumberUtil

core/src/main/scala/org/bitcoins/core/script/interpreter/ScriptInterpreter.scala

@@ -74,7 +74,6 @@ sealed abstract class ScriptInterpreter extends BitcoinSLogger {
             executeProgram(scriptPubKeyProgram)
           logger.trace(
             s"scriptPubKeyExecutedProgram $scriptPubKeyExecutedProgram")
-
           if (scriptSigExecutedProgram.error.isDefined) {
             scriptSigExecutedProgram
           } else if (scriptPubKeyExecutedProgram.error.isDefined || scriptPubKeyExecutedProgram.stackTopIsFalse) {
@@ -382,7 +381,11 @@ sealed abstract class ScriptInterpreter extends BitcoinSLogger {
         evaluated: ExecutedScriptProgram): ExecutedScriptProgram = {
       logger.trace("Stack after evaluating witness: " + evaluated.stack)
       if (evaluated.error.isDefined) evaluated
-      else if (evaluated.stack.size != 1 || evaluated.stackTopIsFalse)
+      else if (evaluated.stack.size != 1) {
+        // Scripts inside witness implicitly require cleanstack behaviour
+        //https://github.com/bitcoin/bitcoin/blob/561a7d30478b82f5d46dcf0f16e864a9608004f4/src/script/interpreter.cpp#L1464
+        evaluated.failExecution(ScriptErrorCleanStack)
+      } else if (evaluated.stackTopIsFalse)
         evaluated.failExecution(ScriptErrorEvalFalse)
       else evaluated
     }