From 8dadfad1aa3b1ade618619850c2e35c0414bdba6 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Mon, 4 Apr 2016 15:26:20 -0500 Subject: [PATCH] Fixing various bugs in Arithmetic interpreter - also fixing bug in ScriptInterpreter for detecting if the return value is false --- .../org/scalacoin/script/ScriptProgram.scala | 4 +- .../arithmetic/ArithmeticInterpreter.scala | 25 +++++------ .../script/constant/ConstantInterpreter.scala | 9 +++- .../interpreter/ScriptInterpreter.scala | 2 +- .../ArithmeticInterpreterTest.scala | 43 +++++++++++++------ .../interpreter/ScriptInterpreterTest.scala | 23 ++-------- 6 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/main/scala/org/scalacoin/script/ScriptProgram.scala b/src/main/scala/org/scalacoin/script/ScriptProgram.scala index c9833c1a50..fa74d09380 100644 --- a/src/main/scala/org/scalacoin/script/ScriptProgram.scala +++ b/src/main/scala/org/scalacoin/script/ScriptProgram.scala @@ -2,7 +2,7 @@ package org.scalacoin.script import org.scalacoin.protocol.script.{ScriptSignature, ScriptPubKey} import org.scalacoin.protocol.transaction.Transaction -import org.scalacoin.script.constant.{OP_0, ScriptNumberImpl, ScriptFalse, ScriptToken} +import org.scalacoin.script.constant._ import org.scalacoin.script.flag.ScriptFlag /** @@ -90,7 +90,7 @@ trait ScriptProgram { */ def stackTopIsFalse : Boolean = { if (stack.headOption.isDefined && - (stack.head == ScriptFalse || stack.head == ScriptNumberImpl(0) || stack.head == OP_0)) true + (stack.head == ScriptFalse || stack.head == ScriptNumberImpl(0) || stack.head == OP_0 || stack.head == OP_FALSE)) true else if (!stack.headOption.isDefined) true else false } diff --git a/src/main/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreter.scala b/src/main/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreter.scala index ae022dfb89..add8581127 100644 --- a/src/main/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreter.scala +++ b/src/main/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreter.scala @@ -118,11 +118,12 @@ trait ArithmeticInterpreter extends ControlOperationsInterpreter { require(program.script.headOption.isDefined && program.script.head == OP_NOT, "Script top must be OP_NOT") require(program.stack.size > 0, "Stack size must be 1 or more perform an OP_NOT") val newStackTop = program.stack.head match { - case OP_0 => OP_1 + case OP_0 => OP_TRUE + case OP_FALSE => OP_TRUE case OP_1 => OP_0 - case ScriptNumberImpl(0) => OP_1 - case ScriptNumberImpl(1) => OP_0 - case _ => OP_0 + case ScriptNumberImpl(0) => OP_TRUE + case ScriptNumberImpl(1) => OP_FALSE + case _ => OP_FALSE } ScriptProgramFactory.factory(program, newStackTop :: program.stack.tail, program.script.tail) } @@ -154,7 +155,9 @@ trait ArithmeticInterpreter extends ControlOperationsInterpreter { require(program.stack.size > 1, "Stack size must be 2 or more perform an OP_BOOLAND") val b = program.stack.head val a = program.stack.tail.head - val newStackTop = if ( b == a && (a == ScriptNumberImpl(0) || a == OP_0)) OP_1 else OP_0 + val aIsFalse = (a == ScriptNumberImpl(0) || a == OP_0) + val bIsFalse = (b == ScriptNumberImpl(0) || b == OP_0) + val newStackTop = if (aIsFalse || bIsFalse) OP_FALSE else OP_TRUE ScriptProgramFactory.factory(program, newStackTop :: program.stack.tail.tail, program.script.tail) } @@ -184,12 +187,13 @@ trait ArithmeticInterpreter extends ControlOperationsInterpreter { val b = program.stack.head val a = program.stack.tail.head val isSame = (a,b) match { + case (x : ScriptNumberOperation, y : ScriptNumberOperation) => x == y case (x : ScriptNumberOperation, y : ScriptNumber) => x.scriptNumber == y case (x : ScriptNumber, y : ScriptNumberOperation) => x == y.scriptNumber case (x,y) => x == y } - val newStackTop = if (isSame) OP_1 else OP_0 + val newStackTop = if (isSame) OP_TRUE else OP_FALSE ScriptProgramFactory.factory(program, newStackTop :: program.stack.tail.tail, program.script.tail) } @@ -386,14 +390,11 @@ trait ArithmeticInterpreter extends ControlOperationsInterpreter { val a = program.stack.tail.tail.head val isWithinRange = (a,b,c) match { - case (x : ScriptNumberOperation, y : ScriptNumber, z : ScriptNumber) => z >= x && z < y - case (x : ScriptNumber, y : ScriptNumberOperation, z : ScriptNumber) => z >= x && z < y.scriptNumber - case (x : ScriptNumber, y : ScriptNumber, z : ScriptNumberOperation) => z.scriptNumber >= x && z.scriptNumber < y - case (x : ScriptNumber, y : ScriptNumber, z : ScriptNumber) => z >= x && z < y - case (x,y,z) => z.toLong >= x.toLong && z.toLong < y.toLong + case (x : ScriptNumber, y : ScriptNumber, z : ScriptNumber) => x >= y && x < z + case (x,y,z) => x.toLong >= y.toLong && x.toLong < z.toLong } - val newStackTop = if (isWithinRange) OP_1 else OP_0 + val newStackTop = if (isWithinRange) OP_TRUE else OP_FALSE ScriptProgramFactory.factory(program, newStackTop :: program.stack.tail.tail.tail, program.script.tail) } diff --git a/src/main/scala/org/scalacoin/script/constant/ConstantInterpreter.scala b/src/main/scala/org/scalacoin/script/constant/ConstantInterpreter.scala index 88cb252f6d..ba19be5474 100644 --- a/src/main/scala/org/scalacoin/script/constant/ConstantInterpreter.scala +++ b/src/main/scala/org/scalacoin/script/constant/ConstantInterpreter.scala @@ -1,6 +1,7 @@ package org.scalacoin.script.constant import org.scalacoin.script.{ScriptProgramFactory, ScriptProgramImpl, ScriptProgram} +import org.scalacoin.util.BitcoinSUtil import org.slf4j.LoggerFactory import scala.annotation.tailrec @@ -108,7 +109,13 @@ trait ConstantInterpreter { } val (newScript,bytesToPushOntoStack) = takeUntilBytesNeeded(program.script.tail,List()) - ScriptProgramFactory.factory(program, bytesToPushOntoStack ++ program.stack, newScript) + logger.debug("new script: " + newScript) + logger.debug("Bytes to push onto stack" + bytesToPushOntoStack) + val constant : ScriptToken = if (bytesToPushOntoStack.size == 1) bytesToPushOntoStack.head + else ScriptConstantFactory.fromHex(BitcoinSUtil.flipEndianess(bytesToPushOntoStack.flatMap(_.bytes))) + + logger.debug("Constant: " + constant) + ScriptProgramFactory.factory(program, constant :: program.stack, newScript) } diff --git a/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala b/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala index 25a7a83075..bd101a6cd7 100644 --- a/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala +++ b/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala @@ -151,7 +151,7 @@ trait ScriptInterpreter extends CryptoInterpreter with StackInterpreter with Con //in this case, just remove OP_CLTV from the stack and continue else loop(ScriptProgramFactory.factory(program, program.script.tail, ScriptProgramFactory.Script)) //no more script operations to run, True is represented by any representation of non-zero - case Nil => (program.stack.headOption != Some(ScriptFalse), program) + case Nil => (program.stackTopIsTrue, program) case h :: t => throw new RuntimeException(h + " was unmatched") } diff --git a/src/test/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreterTest.scala b/src/test/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreterTest.scala index aaff54c746..f6619bb109 100644 --- a/src/test/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreterTest.scala +++ b/src/test/scala/org/scalacoin/script/arithmetic/ArithmeticInterpreterTest.scala @@ -1,7 +1,7 @@ package org.scalacoin.script.arithmetic import org.scalacoin.script.{ScriptProgramFactory, ScriptProgramImpl} -import org.scalacoin.script.constant.{OP_0, OP_1, ScriptConstantImpl, ScriptNumberImpl} +import org.scalacoin.script.constant._ import org.scalacoin.util.TestUtil import org.scalatest.{FlatSpec, MustMatchers} @@ -219,7 +219,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opNot(program) - newProgram.stack.head must be (OP_1) + newProgram.stackTopIsTrue must be (true) + newProgram.stack.head must be (OP_TRUE) newProgram.script.isEmpty must be (true) } @@ -229,7 +230,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opNot(program) - newProgram.stack.head must be (OP_0) + newProgram.stackTopIsFalse must be (true) + newProgram.stack.head must be (OP_FALSE) newProgram.script.isEmpty must be (true) } @@ -259,7 +261,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opBoolAnd(program) - newProgram.stack.head must be (OP_1) + newProgram.stackTopIsFalse must be (true) + newProgram.stack.head must be (OP_FALSE) newProgram.script.isEmpty must be (true) val stack1 = List(OP_0, OP_0) @@ -267,7 +270,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program1 = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram1 = opBoolAnd(program) - newProgram1.stack.head must be (OP_1) + newProgram.stackTopIsFalse must be (true) + newProgram1.stack.head must be (OP_FALSE) newProgram1.script.isEmpty must be (true) } @@ -277,7 +281,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opBoolAnd(program) - newProgram.stack.head must be (OP_0) + newProgram.stackTopIsTrue must be (false) + newProgram.stack.head must be (OP_FALSE) newProgram.script.isEmpty must be (true) } @@ -287,7 +292,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opBoolAnd(program) - newProgram.stack.head must be (OP_0) + newProgram.stackTopIsTrue must be (true) + newProgram.stack.head must be (OP_TRUE) newProgram.script.isEmpty must be (true) } @@ -327,7 +333,18 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opNumEqual(program) - newProgram.stack.head must be (OP_1) + newProgram.stack.head must be (OP_TRUE) + newProgram.script.isEmpty must be (true) + } + + it must "evaulate an OP_NUMEQUAL for two OP_0" in { + val stack = List(OP_0, OP_0) + val script = List(OP_NUMEQUAL) + val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) + val newProgram = opNumEqual(program) + + newProgram.stackTopIsTrue must be (true) + newProgram.stack.head must be (OP_TRUE) newProgram.script.isEmpty must be (true) } @@ -341,6 +358,8 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet newProgram.script.isEmpty must be (true) } + + it must "evaluate an OP_NUMNOTEQUAL for two numbers that are not the same" in { val stack = List(OP_0, ScriptNumberImpl(1)) val script = List(OP_NUMNOTEQUAL) @@ -424,19 +443,19 @@ class ArithmeticInterpreterTest extends FlatSpec with MustMatchers with Arithmet } it must "evaluate an OP_WITHIN correctly" in { - val stack = List(OP_0,ScriptNumberImpl(2), ScriptNumberImpl(1)) + val stack = List(ScriptNumberImpl(2), ScriptNumberImpl(1), OP_0) val script = List(OP_WITHIN) val program = ScriptProgramFactory.factory(TestUtil.testProgram, stack,script) val newProgram = opWithin(program) - newProgram.stack must be (List(OP_0)) + newProgram.stack must be (List(OP_FALSE)) newProgram.script.isEmpty must be (true) - val stack1 = List(OP_0, ScriptNumberImpl(1),ScriptNumberImpl(0)) + val stack1 = List(ScriptNumberImpl(1), OP_0, ScriptNumberImpl(0)) val script1 = List(OP_WITHIN) val program1 = ScriptProgramFactory.factory(TestUtil.testProgram, stack1,script1) val newProgram1 = opWithin(program1) - newProgram1.stack must be (List(OP_1)) + newProgram1.stack must be (List(OP_TRUE)) newProgram1.script.isEmpty must be (true) } diff --git a/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala b/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala index 7d200b4fe3..8f89e42d8e 100644 --- a/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala +++ b/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala @@ -53,17 +53,8 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp run(program) must equal (true) } - it must "evaluate a NOP correctly" in { - val stack = List() - val script = List(OP_NOP) - val program = ScriptProgramFactory.factory(TestUtil.testProgram,stack,script) - run(program) must equal (true) - - } - -/* it must "evaluate all valid scripts from the bitcoin core script_valid.json" in { import CoreTestCaseProtocol._ @@ -106,9 +97,9 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp } } - }*/ + } - it must "evaluate all valid scripts from the bitcoin core script_invalid.json" in { +/* it must "evaluate all valid scripts from the bitcoin core script_invalid.json" in { import CoreTestCaseProtocol._ /** @@ -129,21 +120,15 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp val lines = try source.getLines.filterNot(_.isEmpty).map(_.trim) mkString "\n" finally source.close() - logger.info("1") val json = lines.parseJson - logger.info("2") val testCasesOpt : Seq[Option[CoreTestCase]] = json.convertTo[Seq[Option[CoreTestCase]]] - logger.info("3") val testCases : Seq[CoreTestCase] = testCasesOpt.flatten - logger.info("4") + for { testCase <- testCases - _ = logger.info(testCase.toString) (creditingTx,outputIndex) = TransactionTestUtil.buildCreditingTransaction(testCase.scriptPubKey.scriptPubKey) - _ = logger.info("6") (tx,inputIndex) = TransactionTestUtil.buildSpendingTransaction(creditingTx,testCase.scriptSig.scriptSignature,outputIndex) - _ = logger.info("7") } yield { require(testCase.scriptPubKey.asm == testCase.scriptPubKey.scriptPubKey.asm) @@ -162,5 +147,5 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp } } - } + }*/ }