Moved BitcoinTxBuilder property tests into ScalaTest context, fixed bug where opPushData was marking valid short P2SH scripts as invalid (#900)

This commit is contained in:
Nadav Kohen 2019-11-27 12:51:49 -07:00 committed by Chris Stewart
parent 15ec7b89b6
commit b89d20e9a8
5 changed files with 211 additions and 241 deletions

View file

@ -1,191 +0,0 @@
package org.bitcoins.core.wallet.builder
import org.bitcoins.core.crypto.{
BaseTxSigComponent,
WitnessTxSigComponentP2SH,
WitnessTxSigComponentRaw
}
import org.bitcoins.core.currency.{CurrencyUnits, Satoshis}
import org.bitcoins.testkit.core.gen.{
ChainParamsGenerator,
CreditingTxGen,
ScriptGenerators,
TransactionGenerators
}
import org.bitcoins.core.number.{Int64, UInt32}
import org.bitcoins.core.policy.Policy
import org.bitcoins.core.protocol.script._
import org.bitcoins.core.protocol.transaction._
import org.bitcoins.core.script.PreExecutionScriptProgram
import org.bitcoins.core.script.interpreter.ScriptInterpreter
import org.bitcoins.core.util.BitcoinSLogger
import org.bitcoins.core.wallet.fee.{SatoshisPerByte, SatoshisPerVirtualByte}
import org.bitcoins.core.wallet.utxo.{BitcoinUTXOSpendingInfo, UTXOSpendingInfo}
import org.scalacheck.{Prop, Properties}
import scala.annotation.tailrec
import scala.concurrent.Await
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.DurationInt
import scala.util.Try
class BitcoinTxBuilderSpec extends Properties("TxBuilderSpec") {
private val logger = BitcoinSLogger.logger
private val tc = TransactionConstants
val timeout = 10.seconds
property("sign a mix of spks in a tx and then have it verified") = {
Prop.forAllNoShrink(CreditingTxGen.outputs) {
case creditingTxsInfo =>
val creditingOutputs = creditingTxsInfo.map(c => c.output)
val creditingOutputsAmt = creditingOutputs.map(_.value)
val totalAmount = creditingOutputsAmt.fold(CurrencyUnits.zero)(_ + _)
Prop.forAllNoShrink(TransactionGenerators.smallOutputs(totalAmount),
ScriptGenerators.scriptPubKey,
ChainParamsGenerator.bitcoinNetworkParams) {
case (destinations: Seq[TransactionOutput], changeSPK, network) =>
val fee = SatoshisPerVirtualByte(Satoshis(1000))
val outpointsWithKeys =
buildCreditingTxInfo(creditingTxsInfo.toList)
val builder = BitcoinTxBuilder(destinations,
outpointsWithKeys,
fee,
changeSPK._1,
network)
val tx = Await.result(builder.flatMap(_.sign), timeout)
verifyScript(tx, creditingTxsInfo)
}
}
}
property("sign a mix of p2sh/p2wsh in a tx and then have it verified") = {
Prop.forAllNoShrink(CreditingTxGen.nestedOutputs) {
case creditingTxsInfo =>
val creditingOutputs = creditingTxsInfo.map(c => c.output)
val creditingOutputsAmt = creditingOutputs.map(_.value)
val totalAmount = creditingOutputsAmt.fold(CurrencyUnits.zero)(_ + _)
Prop.forAll(TransactionGenerators.smallOutputs(totalAmount),
ScriptGenerators.scriptPubKey,
ChainParamsGenerator.bitcoinNetworkParams) {
case (destinations: Seq[TransactionOutput], changeSPK, network) =>
val fee = SatoshisPerByte(Satoshis(1000))
val outpointsWithKeys =
buildCreditingTxInfo(creditingTxsInfo.toList)
val builder = BitcoinTxBuilder(destinations,
outpointsWithKeys,
fee,
changeSPK._1,
network)
val tx = Await.result(builder.flatMap(_.sign), timeout)
verifyScript(tx, creditingTxsInfo)
}
}
}
/*
property("random fuzz test for tx builder") = {
Prop.forAllNoShrink(CreditingTxGen.randoms) {
case creditingTxsInfo =>
val creditingOutputs = creditingTxsInfo.map(c => c.output)
val creditingOutputsAmt = creditingOutputs.map(_.value)
val totalAmount = creditingOutputsAmt.fold(CurrencyUnits.zero)(_ + _)
Prop.forAllNoShrink(TransactionGenerators.smallOutputs(totalAmount),
ScriptGenerators.scriptPubKey,
ChainParamsGenerator.bitcoinNetworkParams) {
case (destinations: Seq[TransactionOutput], changeSPK, network) =>
val fee = SatoshisPerVirtualByte(Satoshis(Int64(1000)))
val outpointsWithKeys =
buildCreditingTxInfo(creditingTxsInfo.toList)
val builder = BitcoinTxBuilder(destinations,
outpointsWithKeys,
fee,
changeSPK._1,
network)
val result = Try(Await.result(builder.flatMap(_.sign), timeout))
if (result.isFailure) true
else !verifyScript(result.get, creditingTxsInfo)
}
}
}
*/
private def buildCreditingTxInfo(
info: List[BitcoinUTXOSpendingInfo]): BitcoinTxBuilder.UTXOMap = {
@tailrec
def loop(rem: List[BitcoinUTXOSpendingInfo],
accum: BitcoinTxBuilder.UTXOMap): BitcoinTxBuilder.UTXOMap =
rem match {
case Nil => accum
case BitcoinUTXOSpendingInfo(txOutPoint,
txOutput,
signers,
redeemScriptOpt,
scriptWitOpt,
hashType,
conditionalPath) :: t =>
val o = txOutPoint
val output = txOutput
val outPointsSpendingInfo = BitcoinUTXOSpendingInfo(o,
output,
signers,
redeemScriptOpt,
scriptWitOpt,
hashType,
conditionalPath)
loop(t, accum.updated(o, outPointsSpendingInfo))
}
loop(info, Map.empty)
}
def verifyScript(tx: Transaction, utxos: Seq[UTXOSpendingInfo]): Boolean = {
val programs: Seq[PreExecutionScriptProgram] = tx.inputs.zipWithIndex.map {
case (input: TransactionInput, idx: Int) =>
val outpoint = input.previousOutput
val creditingTx = utxos.find(u => u.outPoint.txId == outpoint.txId).get
val output = creditingTx.output
val spk = output.scriptPubKey
val amount = output.value
val txSigComponent = spk match {
case witSPK: WitnessScriptPubKeyV0 =>
val o = TransactionOutput(amount, witSPK)
WitnessTxSigComponentRaw(tx.asInstanceOf[WitnessTransaction],
UInt32(idx),
o,
Policy.standardFlags)
case _: UnassignedWitnessScriptPubKey => ???
case x @ (_: P2PKScriptPubKey | _: P2PKHScriptPubKey |
_: MultiSignatureScriptPubKey | _: WitnessCommitment |
_: CSVScriptPubKey | _: CLTVScriptPubKey |
_: ConditionalScriptPubKey | _: NonStandardScriptPubKey |
EmptyScriptPubKey) =>
val o = TransactionOutput(CurrencyUnits.zero, x)
BaseTxSigComponent(tx, UInt32(idx), o, Policy.standardFlags)
case p2sh: P2SHScriptPubKey =>
val p2shScriptSig =
tx.inputs(idx).scriptSignature.asInstanceOf[P2SHScriptSignature]
p2shScriptSig.redeemScript match {
case _: WitnessScriptPubKey =>
WitnessTxSigComponentP2SH(transaction =
tx.asInstanceOf[WitnessTransaction],
inputIndex = UInt32(idx),
output = output,
flags = Policy.standardFlags)
case _ =>
BaseTxSigComponent(tx,
UInt32(idx),
output,
Policy.standardFlags)
}
}
PreExecutionScriptProgram(txSigComponent)
}
ScriptInterpreter.runAllVerify(programs)
}
}

View file

@ -1,26 +1,48 @@
package org.bitcoins.core.wallet.builder
import org.bitcoins.core.config.TestNet3
import org.bitcoins.core.crypto.ECPrivateKey
import org.bitcoins.core.crypto.{
BaseTxSigComponent,
ECPrivateKey,
WitnessTxSigComponentP2SH,
WitnessTxSigComponentRaw
}
import org.bitcoins.core.currency._
import org.bitcoins.testkit.core.gen.ScriptGenerators
import org.bitcoins.testkit.core.gen.{
ChainParamsGenerator,
CreditingTxGen,
ScriptGenerators,
TransactionGenerators
}
import org.bitcoins.core.number.{Int64, UInt32}
import org.bitcoins.core.protocol.script._
import org.bitcoins.core.protocol.transaction._
import org.bitcoins.core.script.crypto.HashType
import org.bitcoins.core.util.BitcoinSLogger
import org.bitcoins.core.wallet.fee.SatoshisPerVirtualByte
import org.bitcoins.core.wallet.utxo.{BitcoinUTXOSpendingInfo, ConditionalPath}
import org.scalatest.{AsyncFlatSpec, MustMatchers}
import org.bitcoins.core.wallet.utxo.{
BitcoinUTXOSpendingInfo,
ConditionalPath,
UTXOSpendingInfo
}
import org.bitcoins.core.wallet.fee.SatoshisPerByte
import org.bitcoins.core.config.RegTest
import org.bitcoins.core.policy.Policy
import org.bitcoins.core.script.PreExecutionScriptProgram
import org.bitcoins.core.script.interpreter.ScriptInterpreter
import org.bitcoins.core.wallet.builder.BitcoinTxBuilder.UTXOMap
import org.bitcoins.testkit.Implicits._
import org.bitcoins.testkit.util.BitcoinSAsyncTest
class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
private val logger = BitcoinSLogger.logger
import scala.concurrent.Await
import scala.concurrent.duration.DurationInt
class BitcoinTxBuilderTest extends BitcoinSAsyncTest {
val tc = TransactionConstants
val (spk, privKey) = ScriptGenerators.p2pkhScriptPubKey.sampleSome
implicit override val generatorDrivenConfig: PropertyCheckConfiguration =
generatorDrivenConfigNewCode
behavior of "BitcoinTxBuilder"
// We had a matcherror when passing in a vector of UTXOs,
@ -84,7 +106,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
hashType = HashType.sigHashAll,
conditionalPath = ConditionalPath.NoConditionsLeft
)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val feeUnit = SatoshisPerVirtualByte(Satoshis.one)
val txBuilder = BitcoinTxBuilder(destinations = destinations,
utxos = utxoMap,
@ -115,7 +137,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
hashType = HashType.sigHashAll,
conditionalPath = ConditionalPath.NoConditionsLeft
)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val feeUnit = SatoshisPerVirtualByte(Satoshis(-1))
val txBuilder = BitcoinTxBuilder(destinations = destinations,
utxos = utxoMap,
@ -144,7 +166,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
HashType.sigHashAll,
conditionalPath =
ConditionalPath.NoConditionsLeft)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val feeUnit = SatoshisPerVirtualByte(currencyUnit = Satoshis(1))
val txBuilder = BitcoinTxBuilder(destinations = destinations,
utxos = utxoMap,
@ -181,7 +203,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
hashType = HashType.sigHashAll,
conditionalPath = ConditionalPath.NoConditionsLeft
)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val utxoSpendingInfo = BitcoinUTXOSpendingInfo(
outPoint = outPoint,
output = creditingOutput,
@ -272,7 +294,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
HashType.sigHashAll,
conditionalPath = ConditionalPath.NoConditionsLeft
)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val feeUnit = SatoshisPerVirtualByte(Satoshis.one)
val txBuilderWitness = BitcoinTxBuilder(destinations,
@ -307,7 +329,7 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
hashType = HashType.sigHashAll,
conditionalPath = ConditionalPath.NoConditionsLeft
)
val utxoMap: BitcoinTxBuilder.UTXOMap = Map(outPoint -> utxo)
val utxoMap: UTXOMap = Map(outPoint -> utxo)
val feeUnit = SatoshisPerVirtualByte(Satoshis.one)
val txBuilderWitness = BitcoinTxBuilder(destinations = destinations,
@ -364,4 +386,102 @@ class BitcoinTxBuilderTest extends AsyncFlatSpec with MustMatchers {
)
}
}
def verifyScript(tx: Transaction, utxos: Seq[UTXOSpendingInfo]): Boolean = {
val programs: Seq[PreExecutionScriptProgram] = tx.inputs.zipWithIndex.map {
case (input: TransactionInput, idx: Int) =>
val outpoint = input.previousOutput
val creditingTx = utxos.find(u => u.outPoint.txId == outpoint.txId).get
val output = creditingTx.output
val spk = output.scriptPubKey
val amount = output.value
val txSigComponent = spk match {
case witSPK: WitnessScriptPubKeyV0 =>
val o = TransactionOutput(amount, witSPK)
WitnessTxSigComponentRaw(tx.asInstanceOf[WitnessTransaction],
UInt32(idx),
o,
Policy.standardFlags)
case _: UnassignedWitnessScriptPubKey => ???
case x @ (_: P2PKScriptPubKey | _: P2PKHScriptPubKey |
_: MultiSignatureScriptPubKey | _: WitnessCommitment |
_: CSVScriptPubKey | _: CLTVScriptPubKey |
_: ConditionalScriptPubKey | _: NonStandardScriptPubKey |
EmptyScriptPubKey) =>
val o = TransactionOutput(CurrencyUnits.zero, x)
BaseTxSigComponent(tx, UInt32(idx), o, Policy.standardFlags)
case p2sh: P2SHScriptPubKey =>
val p2shScriptSig =
tx.inputs(idx).scriptSignature.asInstanceOf[P2SHScriptSignature]
p2shScriptSig.redeemScript match {
case _: WitnessScriptPubKey =>
WitnessTxSigComponentP2SH(transaction =
tx.asInstanceOf[WitnessTransaction],
inputIndex = UInt32(idx),
output = output,
flags = Policy.standardFlags)
case _ =>
BaseTxSigComponent(tx,
UInt32(idx),
output,
Policy.standardFlags)
}
}
PreExecutionScriptProgram(txSigComponent)
}
ScriptInterpreter.runAllVerify(programs)
}
private val outputGen = CreditingTxGen.outputs
.flatMap { creditingTxsInfo =>
val creditingOutputs = creditingTxsInfo.map(c => c.output)
val creditingOutputsAmt = creditingOutputs.map(_.value)
val totalAmount = creditingOutputsAmt.fold(CurrencyUnits.zero)(_ + _)
TransactionGenerators.smallOutputs(totalAmount).map { destinations =>
(creditingTxsInfo, destinations)
}
}
.suchThat(_._1.nonEmpty)
it must "sign a mix of spks in a tx and then have it verified" in {
forAll(outputGen,
ScriptGenerators.scriptPubKey,
ChainParamsGenerator.bitcoinNetworkParams) {
case ((creditingTxsInfo, destinations), changeSPK, network) =>
val fee = SatoshisPerVirtualByte(Satoshis(1000))
val builder = BitcoinTxBuilder(destinations,
creditingTxsInfo,
fee,
changeSPK._1,
network)
val tx = Await.result(builder.flatMap(_.sign), 10.seconds)
assert(verifyScript(tx, creditingTxsInfo))
}
}
it must "sign a mix of p2sh/p2wsh in a tx and then have it verified" in {
forAll(outputGen,
ScriptGenerators.scriptPubKey,
ChainParamsGenerator.bitcoinNetworkParams) {
case ((creditingTxsInfo, destinations), changeSPK, network) =>
val fee = SatoshisPerByte(Satoshis(1000))
val builder = BitcoinTxBuilder(destinations,
creditingTxsInfo,
fee,
changeSPK._1,
network)
val tx = Await.result(builder.flatMap(_.sign), 10.seconds)
assert(verifyScript(tx, creditingTxsInfo))
}
}
}

View file

@ -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)
}
}
}

View file

@ -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

View file

@ -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