From 60bb8e0e6bddda5810dfeb3821c6c1573bf16518 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Fri, 22 Jan 2016 13:23:12 -0600 Subject: [PATCH] refactoring the parsing algorithm into functions that can be reused across data types --- .../marshallers/script/ScriptParser.scala | 94 +++++++++++++--- .../scalacoin/script/constant/Constants.scala | 105 ++++++++++++------ .../interpreter/ScriptInterpreter.scala | 9 +- .../script/constant/ConstantsTest.scala | 20 +++- .../constant/ScriptNumberFactoryTest.scala | 4 +- .../interpreter/ScriptInterpreterTest.scala | 11 +- 6 files changed, 183 insertions(+), 60 deletions(-) rename src/{main => test}/scala/org/scalacoin/script/constant/ConstantsTest.scala (70%) diff --git a/src/main/scala/org/scalacoin/marshallers/script/ScriptParser.scala b/src/main/scala/org/scalacoin/marshallers/script/ScriptParser.scala index 3f9e1f48df..d6e9ad409a 100644 --- a/src/main/scala/org/scalacoin/marshallers/script/ScriptParser.scala +++ b/src/main/scala/org/scalacoin/marshallers/script/ScriptParser.scala @@ -16,6 +16,7 @@ trait ScriptParser extends ScalacoinUtil { /** * Parses an asm output script of a transaction * example: "OP_DUP OP_HASH160 e2e7c1ab3f807151e832dd1accb3d4f5d7d19b4b OP_EQUALVERIFY OP_CHECKSIG" + * example: ["0", "IF 0x50 ENDIF 1", "P2SH,STRICTENC", "0x50 is reserved (ok if not executed)"] (from script_valid.json) * @param str * @return */ @@ -26,13 +27,21 @@ trait ScriptParser extends ScalacoinUtil { def loop(operations : List[String], accum : List[ScriptToken]) : List[ScriptToken] = { logger.debug("Attempting to parse: " + operations.headOption) operations match { - //if we see a byte constant in the form of "0x09" + //if we see a byte constant of just 0x09 + //parse the two characters as a hex op + case h :: t if (h.size == 4 && h.substring(0,2) == "0x") => + val hexString = h.substring(2,h.size) + loop(t,ScriptOperationFactory.fromHex(hexString).get :: accum) + //if we see a byte constant in the form of "0x09adb" case h :: t if (h.size > 1 && h.substring(0,2) == "0x") => loop(t,parseBytesFromString(h) ++ accum) //skip the empty string case h :: t if (h == "") => loop(t,accum) case h :: t if (h == "0") => loop(t, OP_0 :: accum) case h :: t if (ScriptOperationFactory.fromString(h).isDefined) => - loop(t,ScriptOperationFactory.fromString(h).get :: accum) + val op = ScriptOperationFactory.fromString(h).get + val parsingHelper : ParsingHelper[String] = parseOperationString(op,accum,t) + loop(parsingHelper.tail,parsingHelper.accum) + case h :: t => loop(t, ScriptConstantImpl(h) :: accum) case Nil => accum } @@ -68,30 +77,29 @@ trait ScriptParser extends ScalacoinUtil { logger.debug("Byte to be parsed: " + bytes.headOption) bytes match { case h :: t => - logger.debug("Op for matching: " + h) val op = ScriptOperationFactory.fromByte(h).get - logger.debug("Matched op: " + op) - op match { - case scriptNumber : ScriptNumberImpl => - //means that we need to push x amount of bytes on to the stack - val (constant,tail) = pushConstant(scriptNumber,t) - loop(tail, constant :: accum) - case _ => - //means that we need to push the operation onto the stack - loop(t, op :: accum) - } + val parsingHelper : ParsingHelper[Byte] = parseOperationByte(op,accum,t) + loop(parsingHelper.tail,parsingHelper.accum) case Nil => accum } } loop(bytes, List()).reverse } - private def pushConstant(op : ScriptNumber, bytes : List[Byte]) : (ScriptConstant, List[Byte]) = { + /** + * Slices the amount of bytes specified in the op parameter and then creates a script constant + * from those bytes. Returns the script constant and the byte array without the script constant + * @param op + * @param bytes + * @return + */ + private def sliceConstant[T](op : ScriptNumber, data : List[T]) : (List[T], List[T]) = { val finalIndex = op.opCode - val constant : ScriptConstantImpl = ScriptConstantImpl(encodeHex(bytes.slice(0,finalIndex))) - (constant, bytes.slice(finalIndex,bytes.size)) + val dataConstant = data.slice(0,finalIndex) + (dataConstant,data.slice(finalIndex,data.size)) } + /** * Parses the bytes in string format, an example input would look like this * "0x09 0x00000000 0x00000000 0x10" @@ -106,11 +114,63 @@ trait ScriptParser extends ScalacoinUtil { .map(g => BigInt(g.group(1), 16).toString(16)) .toList) val paddedHexStrings = hexStrings.map(hex => if (hex.size == 1) "0"+hex else hex ) - logger.debug("Padded hex strings: " + paddedHexStrings) + //logger.debug("Padded hex strings: " + paddedHexStrings) //TODO: Figure out a better way to do this without calling .get on the result of fromByte val constants = paddedHexStrings.map(ScriptConstantImpl(_)) constants } + + + case class ParsingHelper[T](tail : List[T], accum : List[ScriptToken]) + /** + * Parses an operation if the tail is a List[Byte] + * If the operation is a script number, it pushes the number of bytes onto the stack + * specified by the script number + * i.e. If the operation was ScriptNumber(5), it would slice 5 bytes off of the tail and + * places them into a ScriptConstant and add them to the accumulator. + * @param op + * @param accum + * @param tail + * @return + */ + private def parseOperationByte(op : ScriptOperation, accum : List[ScriptToken], tail : List[Byte]) : ParsingHelper[Byte] = { + op match { + case scriptNumber : ScriptNumberImpl => + //means that we need to push x amount of bytes on to the stack + val (constant,newTail) = sliceConstant(scriptNumber,tail) + val scriptConstant = new ScriptConstantImpl(constant) + ParsingHelper(newTail,scriptConstant :: accum) + + case _ => + //means that we need to push the operation onto the stack + ParsingHelper(tail,op :: accum) + } + } + + /** + * Parses an operation if the tail is a List[String] + * If the operation is a script number, it pushes the number of bytes onto the stack + * specified by the script number + * i.e. If the operation was ScriptNumber(5), it would slice 5 bytes off of the tail and + * places them into a ScriptConstant and add them to the accumulator. + * @param op + * @param accum + * @param tail + * @return + */ + private def parseOperationString(op : ScriptOperation, accum : List[ScriptToken], tail : List[String]) : ParsingHelper[String] = { + op match { + case scriptNumber : ScriptNumberImpl => + //means that we need to push x amount of bytes on to the stack + val (constant,newTail) = sliceConstant[String](scriptNumber,tail) + val scriptConstant = ScriptConstantImpl(constant.mkString) + ParsingHelper(newTail,scriptConstant :: accum) + + case _ => + //means that we need to push the operation onto the stack + ParsingHelper(tail,op :: accum) + } + } } object ScriptParser extends ScriptParser diff --git a/src/main/scala/org/scalacoin/script/constant/Constants.scala b/src/main/scala/org/scalacoin/script/constant/Constants.scala index 4d63b71a03..c9e0e80c89 100644 --- a/src/main/scala/org/scalacoin/script/constant/Constants.scala +++ b/src/main/scala/org/scalacoin/script/constant/Constants.scala @@ -18,34 +18,24 @@ trait ScriptOperation extends ScriptToken { sealed trait ScriptConstant extends ScriptToken -sealed trait ScriptBoolean extends ScriptConstant +sealed trait ScriptBoolean extends ScriptNumber case object ScriptTrue extends ScriptBoolean { - override def hex = "1" + override def opCode = 1 } case object ScriptFalse extends ScriptBoolean { - override def hex = "0" + override def opCode = 0 } /** * Represent a pubkey or hash of a pub key on our stack * @param hex */ -case class ScriptConstantImpl(hex : String) extends ScriptConstant +case class ScriptConstantImpl(hex : String) extends ScriptConstant { + def this(bytes : List[Byte]) = this(ScalacoinUtil.encodeHex(bytes)) +} -/** - * An empty array of bytes is pushed onto the stack. (This is not a no-op: an item is added to the stack.) - */ -case object OP_0 extends ScriptOperation with ScriptNumber { - override def opCode = 0 -} -/** - * An empty array of bytes is pushed onto the stack. (This is not a no-op: an item is added to the stack.) - */ -case object OP_FALSE extends ScriptOperation { - override def opCode = OP_0.opCode -} /** * The next byte contains the number of bytes to be pushed onto the stack. @@ -68,130 +58,177 @@ case object OP_PUSHDATA4 extends ScriptOperation { override def opCode = 78 } + +/** + * Represents a script number operation where the the number in the operation is pushed onto the stack + * i.e. OP_0 would be push 0 onto the stack, OP_1 would be push 1 onto the stack + */ +sealed trait ScriptNumberOperation extends ScriptNumber { + + /** + * Represents the script number that needs to be pushed onto the stack + * if the op is interpreted + * i.e. OP_1 would be matched with the ScriptNumber(1) + * @return + */ + def scriptNumber : ScriptNumber +} +/** + * An empty array of bytes is pushed onto the stack. (This is not a no-op: an item is added to the stack.) + */ +case object OP_0 extends ScriptNumberOperation { + override def opCode = 0 + override def scriptNumber = this +} +/** + * An empty array of bytes is pushed onto the stack. (This is not a no-op: an item is added to the stack.) + */ +case object OP_FALSE extends ScriptNumberOperation { + override def opCode = OP_0.opCode + override def scriptNumber = OP_0.scriptNumber +} /** * The number -1 is pushed onto the stack. */ -case object OP_1NEGATE extends ScriptOperation { +case object OP_1NEGATE extends ScriptNumberOperation { override def opCode = 79 + override def scriptNumber = ScriptNumberFactory.factory(-1).get } /** * The number 1 is pushed onto the stack. */ -case object OP_TRUE extends ScriptOperation { +case object OP_TRUE extends ScriptNumberOperation { override def opCode = 81 + override def scriptNumber = ScriptNumberFactory.factory(1).get } /** * The number 1 is pushed onto the stack. */ -case object OP_1 extends ScriptOperation with ScriptNumber { +case object OP_1 extends ScriptNumberOperation { override def opCode = OP_TRUE.opCode + override def scriptNumber = ScriptNumberFactory.factory(1).get } /** * The number 2 is pushed onto the stack. */ -case object OP_2 extends ScriptOperation with ScriptNumber { +case object OP_2 extends ScriptNumberOperation { override def opCode = 82 + override def scriptNumber = ScriptNumberFactory.factory(2).get } /** * The number 3 is pushed onto the stack. */ -case object OP_3 extends ScriptOperation with ScriptNumber { +case object OP_3 extends ScriptNumberOperation { override def opCode = 83 + override def scriptNumber = ScriptNumberFactory.factory(3).get } /** * The number 4 is pushed onto the stack. */ -case object OP_4 extends ScriptOperation with ScriptNumber { +case object OP_4 extends ScriptNumberOperation { override def opCode = 84 + override def scriptNumber = ScriptNumberFactory.factory(4).get } /** * The number 5 is pushed onto the stack. */ -case object OP_5 extends ScriptOperation with ScriptNumber { +case object OP_5 extends ScriptNumberOperation { override def opCode = 85 + override def scriptNumber = ScriptNumberFactory.factory(5).get } /** * The number 6 is pushed onto the stack. */ -case object OP_6 extends ScriptOperation with ScriptNumber { +case object OP_6 extends ScriptNumberOperation { override def opCode = 86 + override def scriptNumber = ScriptNumberFactory.factory(6).get } /** * The number 7 is pushed onto the stack. */ -case object OP_7 extends ScriptOperation with ScriptNumber { +case object OP_7 extends ScriptNumberOperation { override def opCode = 87 + override def scriptNumber = ScriptNumberFactory.factory(7).get } /** * The number 8 is pushed onto the stack. */ -case object OP_8 extends ScriptOperation with ScriptNumber { +case object OP_8 extends ScriptNumberOperation { override def opCode = 88 + override def scriptNumber = ScriptNumberFactory.factory(8).get } /** * The number 9 is pushed onto the stack. */ -case object OP_9 extends ScriptOperation with ScriptNumber { +case object OP_9 extends ScriptNumberOperation { override def opCode = 89 + override def scriptNumber = ScriptNumberFactory.factory(9).get } /** * The number 10 is pushed onto the stack. */ -case object OP_10 extends ScriptOperation with ScriptNumber { +case object OP_10 extends ScriptNumberOperation { override def opCode = 90 + override def scriptNumber = ScriptNumberFactory.factory(10).get } /** * The number 11 is pushed onto the stack. */ -case object OP_11 extends ScriptOperation with ScriptNumber { +case object OP_11 extends ScriptNumberOperation { override def opCode = 91 + override def scriptNumber = ScriptNumberFactory.factory(11).get } /** * The number 12 is pushed onto the stack. */ -case object OP_12 extends ScriptOperation with ScriptNumber { +case object OP_12 extends ScriptNumberOperation { override def opCode = 92 + override def scriptNumber = ScriptNumberFactory.factory(12).get } /** * The number 13 is pushed onto the stack. */ -case object OP_13 extends ScriptOperation with ScriptNumber { +case object OP_13 extends ScriptNumberOperation { override def opCode = 93 + override def scriptNumber = ScriptNumberFactory.factory(13).get } /** * The number 14 is pushed onto the stack. */ -case object OP_14 extends ScriptOperation with ScriptNumber { +case object OP_14 extends ScriptNumberOperation { override def opCode = 94 + override def scriptNumber = ScriptNumberFactory.factory(14).get } /** * The number 15 is pushed onto the stack. */ -case object OP_15 extends ScriptOperation with ScriptNumber { +case object OP_15 extends ScriptNumberOperation { override def opCode = 95 + override def scriptNumber = ScriptNumberFactory.factory(15).get } /** * The number 16 is pushed onto the stack. */ -case object OP_16 extends ScriptOperation with ScriptNumber { +case object OP_16 extends ScriptNumberOperation { override def opCode = 96 + override def scriptNumber = ScriptNumberFactory.factory(16).get } diff --git a/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala b/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala index 881191a0cb..bd30a3bf33 100644 --- a/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala +++ b/src/main/scala/org/scalacoin/script/interpreter/ScriptInterpreter.scala @@ -32,6 +32,8 @@ trait ScriptInterpreter extends CryptoInterpreter with StackInterpreter with Con @tailrec def loop(scripts : (List[ScriptToken], List[ScriptToken])) : Boolean = { val (stack,script) = (scripts._1, scripts._2) + logger.debug("Stack: " +stack) + logger.debug("Script: " + script) script match { //stack operations case OP_DUP :: t => loop(opDup(stack,script)) @@ -40,8 +42,6 @@ trait ScriptInterpreter extends CryptoInterpreter with StackInterpreter with Con //bitwise operations case OP_EQUAL :: t => { val (newStack,newScript) = equal(stack, script) - logger.debug("New stack: " + newStack) - logger.debug("New script: " + newScript) if (newStack.head == ScriptTrue && newScript.size == 0) true else if (newStack.head == ScriptFalse && newScript.size == 0) false else loop(newStack,newScript) @@ -52,6 +52,7 @@ trait ScriptInterpreter extends CryptoInterpreter with StackInterpreter with Con //TODO: Implement these case ScriptConstantImpl(x) :: t if x == "1" => throw new RuntimeException("Not implemented yet") case ScriptConstantImpl(x) :: t if x == "0" => throw new RuntimeException("Not implemented yet") + case (scriptNumberOp : ScriptNumberOperation) :: t => loop(scriptNumberOp.scriptNumber :: stack, t) case (scriptNumber : ScriptNumber) :: t => loop(scriptNumber :: stack, t) //TODO: is this right? I need to just push a constant on the input stack??? @@ -60,8 +61,8 @@ trait ScriptInterpreter extends CryptoInterpreter with StackInterpreter with Con //crypto operations case OP_HASH160 :: t => loop(hash160(stack,script)) case OP_CHECKSIG :: t => checkSig(stack,script,fullScript) - //no more script operations to run, if stack top is true or '1' then it is a valid script - case Nil => stack.head == OP_1 || stack.head == ScriptTrue + //no more script operations to run, True is represented by any representation of non-zero + case Nil => stack.head != ScriptFalse case h :: t => throw new RuntimeException(h + " was unmatched") } } diff --git a/src/main/scala/org/scalacoin/script/constant/ConstantsTest.scala b/src/test/scala/org/scalacoin/script/constant/ConstantsTest.scala similarity index 70% rename from src/main/scala/org/scalacoin/script/constant/ConstantsTest.scala rename to src/test/scala/org/scalacoin/script/constant/ConstantsTest.scala index 7f6ed988d8..ccaa04c515 100644 --- a/src/main/scala/org/scalacoin/script/constant/ConstantsTest.scala +++ b/src/test/scala/org/scalacoin/script/constant/ConstantsTest.scala @@ -9,7 +9,8 @@ class ConstantsTest extends FlatSpec with MustMatchers { "Constants" must "define an OP_FALSE" in { OP_FALSE.opCode must be (0) - OP_FALSE.hex must be ("0") + OP_FALSE.hex must be ("00") + OP_FALSE.scriptNumber must be (OP_0) } it must "define an OP_PUSHDATA1" in { @@ -30,6 +31,7 @@ class ConstantsTest extends FlatSpec with MustMatchers { it must "define an OP_1NEGATE" in { OP_1NEGATE.opCode must be (79) OP_1NEGATE.hex must be ("4f") + OP_1NEGATE.scriptNumber must be (ScriptNumberImpl(-1)) } it must "define an OP_TRUE" in { @@ -40,62 +42,78 @@ class ConstantsTest extends FlatSpec with MustMatchers { it must "define an OP_2" in { OP_2.opCode must be (82) OP_2.hex must be ("52") + OP_2.scriptNumber must be (ScriptNumberImpl(2)) } it must "define an OP_3" in { OP_3.opCode must be (83) OP_3.hex must be ("53") + OP_3.scriptNumber must be (ScriptNumberImpl(3)) } it must "define an OP_4" in { OP_4.opCode must be (84) OP_4.hex must be ("54") + OP_4.scriptNumber must be (ScriptNumberImpl(4)) } it must "define an OP_5" in { OP_5.opCode must be (85) OP_5.hex must be ("55") + OP_5.scriptNumber must be (ScriptNumberImpl(5)) } it must "define an OP_6" in { OP_6.opCode must be (86) OP_6.hex must be ("56") + OP_6.scriptNumber must be (ScriptNumberImpl(6)) } it must "define an OP_7" in { OP_7.opCode must be (87) OP_7.hex must be ("57") + OP_7.scriptNumber must be (ScriptNumberImpl(7)) + } it must "define an OP_8" in { OP_8.opCode must be (88) OP_8.hex must be ("58") + OP_8.scriptNumber must be (ScriptNumberImpl(8)) } it must "define an OP_9" in { OP_9.opCode must be (89) OP_9.hex must be ("59") + OP_9.scriptNumber must be (ScriptNumberImpl(9)) } it must "define an OP_10" in { OP_10.opCode must be (90) OP_10.hex must be ("5a") + OP_10.scriptNumber must be (ScriptNumberImpl(10)) } it must "define an OP_11" in { OP_11.opCode must be (91) OP_11.hex must be ("5b") + OP_11.scriptNumber must be (ScriptNumberImpl(11)) } it must "define an OP_12" in { OP_12.opCode must be (92) OP_12.hex must be ("5c") + OP_12.scriptNumber must be (ScriptNumberImpl(12)) } it must "define an OP_13" in { OP_13.opCode must be (93) OP_13.hex must be ("5d") + OP_13.scriptNumber must be (ScriptNumberImpl(13)) } it must "define an OP_14" in { OP_14.opCode must be (94) OP_14.hex must be ("5e") + OP_14.scriptNumber must be (ScriptNumberImpl(14)) } it must "define an OP_15" in { OP_15.opCode must be (95) OP_15.hex must be ("5f") + OP_15.scriptNumber must be (ScriptNumberImpl(15)) } it must "define an OP_16" in { OP_16.opCode must be (96) OP_16.hex must be ("60") + OP_16.scriptNumber must be (ScriptNumberImpl(16)) } diff --git a/src/test/scala/org/scalacoin/script/constant/ScriptNumberFactoryTest.scala b/src/test/scala/org/scalacoin/script/constant/ScriptNumberFactoryTest.scala index 319e95eefd..e06cfb4b09 100644 --- a/src/test/scala/org/scalacoin/script/constant/ScriptNumberFactoryTest.scala +++ b/src/test/scala/org/scalacoin/script/constant/ScriptNumberFactoryTest.scala @@ -27,9 +27,9 @@ class ScriptNumberFactoryTest extends FlatSpec with MustMatchers with ScriptNumb result.get must be (ScriptNumberImpl(2)) } - it must "not allow creation of the script number -1" in { + it must "not allow creation of the script number -2" in { intercept[IllegalArgumentException] { - operations.exists(_ == ScriptNumberImpl(-1)) must be (false) + operations.exists(_ == ScriptNumberImpl(-2)) must be (false) } } diff --git a/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala b/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala index 9d48c48d35..8ae2f1ce55 100644 --- a/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala +++ b/src/test/scala/org/scalacoin/script/interpreter/ScriptInterpreterTest.scala @@ -34,6 +34,14 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp import CoreTestCaseProtocol._ val source = scala.io.Source.fromFile("src/test/scala/org/scalacoin/script/interpreter/script_valid.json") + + //use this to represent a single test case from script_valid.json + val lines = + """ + | + |[["0", "IF 0x50 ENDIF 1", "P2SH,STRICTENC", "0x50 is reserved (ok if not executed)"]] + """.stripMargin + val lines = try source.getLines.filterNot(_.isEmpty).map(_.trim) mkString "\n" finally source.close() val json = lines.parseJson val testCasesOpt : Seq[Option[CoreTestCase]] = json.convertTo[Seq[Option[CoreTestCase]]] @@ -43,13 +51,12 @@ class ScriptInterpreterTest extends FlatSpec with MustMatchers with ScriptInterp for { testCase <- testCases } yield { - logger.info("Running test case: ") logger.info("Raw test case: " + testCase.raw) logger.info("Parsed ScriptSig: " + testCase.scriptSig) logger.info("Parsed ScriptPubKey: " + testCase.scriptPubKey) logger.info("Flags: " + testCase.flags) logger.info("Comments: " + testCase.comments) - withClue(testCase.comments) { + withClue(testCase.raw) { ScriptInterpreter.run(testCase.scriptSig, testCase.scriptPubKey) must equal (true) } }