diff --git a/core/src/main/scala/org/bitcoins/core/script/constant/ConstantInterpreter.scala b/core/src/main/scala/org/bitcoins/core/script/constant/ConstantInterpreter.scala index 038fab9fb1..fc5fe4a1fb 100644 --- a/core/src/main/scala/org/bitcoins/core/script/constant/ConstantInterpreter.scala +++ b/core/src/main/scala/org/bitcoins/core/script/constant/ConstantInterpreter.scala @@ -118,44 +118,31 @@ sealed abstract class ConstantInterpreter { */ private def opPushData( program: ExecutionInProgressScriptProgram): StartedScriptProgram = { - //for the case when we have the minimal data flag and the bytes to push onto stack is represented by the - //constant telling OP_PUSHDATA how many bytes need to go onto the stack - //for instance OP_PUSHDATA1 OP_0 - val scriptNumOp = if (program.script(1).bytes.nonEmpty) { - val h = program.script(1).bytes.head - ScriptNumberOperation.fromNumber(h.toInt) - } else { - None + //for the case where we have to push 0 bytes onto the stack, which is technically the empty byte vector + def emptyPush(): StartedScriptProgram = { + if (ScriptFlagUtil.requireMinimalData(program.flags)) { + program.failExecution(ScriptErrorMinimalData) + } else { + program.updateStackAndScript(ScriptNumber.zero :: program.stack, + program.script.tail.tail) + } } - if (ScriptFlagUtil.requireMinimalData(program.flags) && program - .script(1) - .bytes - .size == 1 && - scriptNumOp.isDefined) { - logger.error( - "We cannot use an OP_PUSHDATA operation for pushing " + - "a script number operation onto the stack, scriptNumberOperation: " + scriptNumOp) - program.failExecution(ScriptErrorMinimalData) - } else if (ScriptFlagUtil.requireMinimalData(program.flags) && program.script.size > 2 && - !BitcoinScriptUtil.isMinimalPush(program.script.head, - program.script(2))) { - logger.error( - "We are not using the minimal push operation to push the bytes onto the stack for the constant") - program.failExecution(ScriptErrorMinimalData) - } else { - //for the case where we have to push 0 bytes onto the stack, which is technically the empty byte vector - program.script(1) match { - case OP_0 | BytesToPushOntoStack.zero | ScriptNumber.zero | - ScriptNumber.negativeZero => - if (ScriptFlagUtil.requireMinimalData(program.flags)) - program.failExecution(ScriptErrorMinimalData) - else - program.updateStackAndScript(ScriptNumber.zero :: program.stack, - program.script.tail.tail) - case _: ScriptToken => - pushScriptNumberBytesToStack(program.updateScript(program.script)) - } + program.script(1) match { + case OP_0 | BytesToPushOntoStack.zero | ScriptNumber.zero | + ScriptNumber.negativeZero => + emptyPush() + case token: ScriptConstant if token.bytes.toSeq.forall(_ == 0.toByte) => + emptyPush() + case _: ScriptToken => + if (ScriptFlagUtil.requireMinimalData(program.flags) && program.script.size > 2 && !BitcoinScriptUtil + .isMinimalPush(program.script.head, program.script(2))) { + logger.error( + s"Non-minimal push where minimal push was required: $program") + program.failExecution(ScriptErrorMinimalData) + } else { + pushScriptNumberBytesToStack(program) + } } } diff --git a/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSAsyncTest.scala b/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSAsyncTest.scala index 49a0247cd0..6ccb7c3340 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSAsyncTest.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSAsyncTest.scala @@ -6,9 +6,12 @@ import akka.util.Timeout import org.bitcoins.core.config.{NetworkParameters, RegTest} import org.bitcoins.core.protocol.blockchain.ChainParams import org.bitcoins.core.util.BitcoinSLogger +import org.scalacheck.Shrink +import org.scalactic.anyvals.PosInt import org.scalatest._ import org.scalatest.concurrent.AsyncTimeLimitedTests import org.scalatest.time.Span +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import scala.concurrent.ExecutionContext import scala.concurrent.duration.DurationInt @@ -21,6 +24,7 @@ trait BaseAsyncTest extends BeforeAndAfter with BeforeAndAfterAll with MustMatchers + with ScalaCheckPropertyChecks with AsyncTimeLimitedTests with BitcoinSLogger { this: AsyncTestSuite => @@ -43,9 +47,49 @@ trait BaseAsyncTest override lazy val timeLimit: Span = 5.minutes + /** This def ensures that shrinks are disabled for all calls to forAll. + * + * If you want to enable shrinking for a specific test, introduce an + * implicit val into that scope with type Shrink[T] where T is the type + * of the generator you want to enable shrinking on. + */ + implicit def noShrink[T]: Shrink[T] = Shrink.shrinkAny[T] + override def afterAll: Unit = { TestKit.shutdownActorSystem(system, verifySystemShutdown = true) } + + /** The configuration for property based tests in our testing suite + * @see http://www.scalatest.org/user_guide/writing_scalacheck_style_properties + */ + implicit override val generatorDrivenConfig: PropertyCheckConfiguration = { + generatorDriveConfigOldCode + } + + /** Sets the generator driven tests to perform the given amount of execs */ + def customGenDrivenConfig(executions: Int): PropertyCheckConfiguration = { + PropertyCheckConfiguration( + minSuccessful = PosInt.from(executions).get, + minSize = PosInt.from(executions).get, + workers = 2 + ) + } + + /** Property based tests that have been around a long time + * have a less of a chance failing, so execute them less + * @return + */ + def generatorDriveConfigOldCode: PropertyCheckConfiguration = { + customGenDrivenConfig(BitcoinSUnitTest.OLD_CODE_EXECUTIONS) + } + + /** Property based tests that are new have a higher chance of failing + * so execute them more + * @return + */ + def generatorDrivenConfigNewCode: PropertyCheckConfiguration = { + customGenDrivenConfig(BitcoinSUnitTest.NEW_CODE_EXECUTIONS) + } } /** A trait that uses [[AsyncFlatSpec]] to execute tests diff --git a/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSUnitTest.scala b/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSUnitTest.scala index 6cd07772dd..9bda5e01e2 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSUnitTest.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/util/BitcoinSUnitTest.scala @@ -1,11 +1,13 @@ package org.bitcoins.testkit.util import org.bitcoins.core.util.BitcoinSLogger +import org.scalacheck.Shrink import org.scalactic.anyvals.PosInt import org.scalatest.concurrent.TimeLimitedTests import org.scalatest.time.Span import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import org.scalatest.{FlatSpec, MustMatchers} + import scala.concurrent.duration.DurationInt /** A wrapper for boiler plate testing procesures in bitcoin-s */ @@ -18,6 +20,14 @@ abstract class BitcoinSUnitTest override val timeLimit: Span = 120.seconds + /** This def ensures that shrinks are disabled for all calls to forAll. + * + * If you want to enable shrinking for a specific test, introduce an + * implicit val into that scope with type Shrink[T] where T is the type + * of the generator you want to enable shrinking on. + */ + implicit def noShrink[T]: Shrink[T] = Shrink.shrinkAny[T] + /** The configuration for property based tests in our testing suite * @see http://www.scalatest.org/user_guide/writing_scalacheck_style_properties */ @@ -52,7 +62,7 @@ abstract class BitcoinSUnitTest } -private object BitcoinSUnitTest { +object BitcoinSUnitTest { /** The number of times new code * should be executed in a property based test