From ab00fffffc93bb69cb9d0224741c922ac2cd9f84 Mon Sep 17 00:00:00 2001 From: Nadav Kohen Date: Fri, 15 Nov 2019 13:26:21 -0700 Subject: [PATCH] Conditional Signing Tests (#865) * Added ConditionalScriptPubKeys to CreditingTxGen so that we are actually testing Conditional signing now * Responded to code review * Renamed scriptPubKeyTooBig => redeemScriptTooBig --- .../interpreter/ScriptInterpreter.scala | 2 +- .../core/util/BitcoinScriptUtil.scala | 12 +- .../bitcoins/core/wallet/signer/Signer.scala | 27 ++++ .../testkit/core/gen/CreditingTxGen.scala | 133 +++++++++++++++--- .../testkit/core/gen/ScriptGenerators.scala | 29 +++- 5 files changed, 170 insertions(+), 33 deletions(-) diff --git a/core/src/main/scala/org/bitcoins/core/script/interpreter/ScriptInterpreter.scala b/core/src/main/scala/org/bitcoins/core/script/interpreter/ScriptInterpreter.scala index 8dd6d5fac6..d332ffb7bf 100644 --- a/core/src/main/scala/org/bitcoins/core/script/interpreter/ScriptInterpreter.scala +++ b/core/src/main/scala/org/bitcoins/core/script/interpreter/ScriptInterpreter.scala @@ -39,7 +39,7 @@ sealed abstract class ScriptInterpreter extends BitcoinSLogger { private lazy val MAX_SCRIPT_OPS = 201 /** We cannot push an element larger than 520 bytes onto the stack */ - private lazy val MAX_PUSH_SIZE = 520 + val MAX_PUSH_SIZE: Int = 520 /** * Runs an entire script though our script programming language and diff --git a/core/src/main/scala/org/bitcoins/core/util/BitcoinScriptUtil.scala b/core/src/main/scala/org/bitcoins/core/util/BitcoinScriptUtil.scala index 3bf5387975..ffd21fd0ff 100644 --- a/core/src/main/scala/org/bitcoins/core/util/BitcoinScriptUtil.scala +++ b/core/src/main/scala/org/bitcoins/core/util/BitcoinScriptUtil.scala @@ -537,13 +537,11 @@ trait BitcoinScriptUtil extends BitcoinSLogger { /** Since witnesses are not run through the interpreter, replace * `OP_0`/`OP_1` with `ScriptNumber.zero`/`ScriptNumber.one` */ def minimalIfOp(asm: Seq[ScriptToken]): Seq[ScriptToken] = { - if (asm == Nil) asm - else if (asm.last == OP_0) { - asm.dropRight(1) ++ Seq(ScriptNumber.zero) - } else if (asm.last == OP_1) { - asm.dropRight(1) ++ Seq(ScriptNumber.one) - } else asm - + asm.map { + case OP_0 => ScriptNumber.zero + case OP_1 => ScriptNumber.one + case token: ScriptToken => token + } } /** Replaces the [[org.bitcoins.core.script.constant.OP_0 OP_0]] dummy for diff --git a/core/src/main/scala/org/bitcoins/core/wallet/signer/Signer.scala b/core/src/main/scala/org/bitcoins/core/wallet/signer/Signer.scala index 5a64710d3f..804d598b9d 100644 --- a/core/src/main/scala/org/bitcoins/core/wallet/signer/Signer.scala +++ b/core/src/main/scala/org/bitcoins/core/wallet/signer/Signer.scala @@ -11,6 +11,7 @@ import org.bitcoins.core.wallet.builder.TxBuilderError import org.bitcoins.core.wallet.utxo.{ BitcoinUTXOSpendingInfo, ConditionalSpendingInfo, + EmptySpendingInfo, LockTimeSpendingInfo, MultiSignatureSpendingInfo, P2PKHSpendingInfo, @@ -185,6 +186,8 @@ object BitcoinSigner { spendingInfoToSatisfy: UTXOSpendingInfo)( implicit ec: ExecutionContext): Future[TxSigComponent] = { spendingInfoToSatisfy match { + case empty: EmptySpendingInfo => + EmptySigner.sign(spendingInfo, unsignedTx, isDummySignature, empty) case p2pk: P2PKSpendingInfo => P2PKSigner.sign(spendingInfo, unsignedTx, isDummySignature, p2pk) case p2pkh: P2PKHSpendingInfo => @@ -217,6 +220,30 @@ object BitcoinSigner { } } +/** For signing EmptyScriptPubKeys in tests, should probably not be used in real life. */ +sealed abstract class EmptySigner extends BitcoinSigner[EmptySpendingInfo] { + + override def sign( + spendingInfo: UTXOSpendingInfo, + unsignedTx: Transaction, + isDummySignature: Boolean, + spendingInfoToSatisfy: EmptySpendingInfo)( + implicit ec: ExecutionContext): Future[TxSigComponent] = { + val (_, output, inputIndex, _) = relevantInfo(spendingInfo, unsignedTx) + + // This script pushes an OP_TRUE onto the stack, causing a successful spend + val satisfyEmptyScriptSig = + Future.successful(NonStandardScriptSignature("0151")) + + updateScriptSigInSigComponent(unsignedTx, + inputIndex.toInt, + output, + satisfyEmptyScriptSig) + } +} + +object EmptySigner extends EmptySigner + /** Used to sign a [[org.bitcoins.core.protocol.script.P2PKScriptPubKey]] */ sealed abstract class P2PKSigner extends BitcoinSigner[P2PKSpendingInfo] { diff --git a/testkit/src/main/scala/org/bitcoins/testkit/core/gen/CreditingTxGen.scala b/testkit/src/main/scala/org/bitcoins/testkit/core/gen/CreditingTxGen.scala index 7165518a6b..118103f2b2 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/core/gen/CreditingTxGen.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/core/gen/CreditingTxGen.scala @@ -5,9 +5,16 @@ import org.bitcoins.core.number.UInt32 import org.bitcoins.core.protocol.script._ import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.script.crypto.HashType -import org.bitcoins.core.wallet.utxo.{BitcoinUTXOSpendingInfo, ConditionalPath} +import org.bitcoins.core.script.interpreter.ScriptInterpreter +import org.bitcoins.core.wallet.utxo.{ + BitcoinUTXOSpendingInfo, + ConditionalPath, + P2SHNestedSegwitV0UTXOSpendingInfo +} import org.scalacheck.Gen +import scala.annotation.tailrec + sealed abstract class CreditingTxGen { /** Minimum amount of outputs to generate */ @@ -22,15 +29,17 @@ sealed abstract class CreditingTxGen { Gen.listOfN(n, TransactionGenerators.realisticOutput) } - def rawOutput: Gen[BitcoinUTXOSpendingInfo] = { + /** Generator for non-script hash based output */ + def nonSHOutput: Gen[BitcoinUTXOSpendingInfo] = { Gen.oneOf(p2pkOutput, p2pkhOutput, multiSigOutput, /*cltvOutput,*/ csvOutput, + conditionalOutput, p2wpkhOutput) } - def rawOutputs: Gen[Seq[BitcoinUTXOSpendingInfo]] = - Gen.choose(min, max).flatMap(n => Gen.listOfN(n, rawOutput)) + def nonSHOutputs: Gen[Seq[BitcoinUTXOSpendingInfo]] = + Gen.choose(min, max).flatMap(n => Gen.listOfN(n, nonSHOutput)) def basicOutput: Gen[BitcoinUTXOSpendingInfo] = { Gen.oneOf(p2pkOutput, p2pkhOutput, multiSigOutput) @@ -40,15 +49,33 @@ sealed abstract class CreditingTxGen { //note, cannot put a p2wpkh here Gen.oneOf(p2pkOutput, p2pkhOutput, - multiSigOutput, /*cltvOutput,*/ csvOutput) + multiSigOutput, /*cltvOutput,*/ csvOutput, + conditionalOutput) } - def nonP2SHOutput: Gen[BitcoinUTXOSpendingInfo] = { - Gen.oneOf(p2pkOutput, - p2pkhOutput, - multiSigOutput, /*cltvOutput,*/ csvOutput, - p2wpkhOutput, - p2wshOutput) + /** Only for use in constructing P2SH outputs */ + private def nonP2SHOutput: Gen[BitcoinUTXOSpendingInfo] = { + Gen + .oneOf(p2pkOutput, + p2pkhOutput, + multiSigOutput, /*cltvOutput,*/ csvOutput, + conditionalOutput, + p2wpkhOutput, + p2wshOutput) + .suchThat(output => + !ScriptGenerators.redeemScriptTooBig(output.scriptPubKey)) + .suchThat { + case P2SHNestedSegwitV0UTXOSpendingInfo(_, + _, + _, + _, + _, + _, + witness: P2WSHWitnessV0, + _) => + witness.stack.exists(_.length > ScriptInterpreter.MAX_PUSH_SIZE) + case _ => true + } } def output: Gen[BitcoinUTXOSpendingInfo] = @@ -57,6 +84,7 @@ sealed abstract class CreditingTxGen { multiSigOutput, p2shOutput, csvOutput, /*cltvOutput,*/ + conditionalOutput, p2wpkhOutput, p2wshOutput) @@ -98,6 +126,31 @@ sealed abstract class CreditingTxGen { Gen.choose(min, max).flatMap(n => Gen.listOfN(n, multiSigOutput)) } + @tailrec + private def noRelevantCLTV(spk: RawScriptPubKey): Boolean = { + spk match { + case _: CLTVScriptPubKey => false + case csv: CSVScriptPubKey => noRelevantCLTV(csv.nestedScriptPubKey) + case conditional: ConditionalScriptPubKey => + noRelevantCLTV(conditional.trueSPK) + case _: RawScriptPubKey => true + } + } + + def conditionalOutput: Gen[BitcoinUTXOSpendingInfo] = { + ScriptGenerators + .nonLocktimeConditionalScriptPubKey(ScriptGenerators.defaultMaxDepth) + //.suchThat { case (spk, _) => noRelevantCLTV(spk) } + .flatMap { + case (conditional, keys) => + build(conditional, keys, None, None) + } + } + + def conditionalOutputs: Gen[Seq[BitcoinUTXOSpendingInfo]] = { + Gen.choose(min, max).flatMap(n => Gen.listOfN(n, conditionalOutput)) + } + def p2shOutput: Gen[BitcoinUTXOSpendingInfo] = nonP2SHOutput.flatMap { o => CryptoGenerators.hashType.map { hashType => val oldOutput = o.output @@ -111,7 +164,7 @@ sealed abstract class CreditingTxGen { Some(redeemScript), o.scriptWitnessOpt, hashType, - ConditionalPath.NoConditionsLeft + computeAllTrueConditionalPath(redeemScript, None, o.scriptWitnessOpt) ) } } @@ -177,13 +230,17 @@ sealed abstract class CreditingTxGen { def p2wpkhOutputs: Gen[Seq[BitcoinUTXOSpendingInfo]] = Gen.choose(min, max).flatMap(n => Gen.listOfN(n, p2wpkhOutput)) - def p2wshOutput: Gen[BitcoinUTXOSpendingInfo] = nonP2WSHOutput.flatMap { - case BitcoinUTXOSpendingInfo(_, txOutput, signer, _, _, _, _) => - val spk = txOutput.scriptPubKey - val scriptWit = P2WSHWitnessV0(spk) - val witSPK = P2WSHWitnessSPKV0(spk) - build(witSPK, signer, None, Some(scriptWit)) - } + def p2wshOutput: Gen[BitcoinUTXOSpendingInfo] = + nonP2WSHOutput + .suchThat(output => + !ScriptGenerators.redeemScriptTooBig(output.scriptPubKey)) + .flatMap { + case BitcoinUTXOSpendingInfo(_, txOutput, signer, _, _, _, _) => + val spk = txOutput.scriptPubKey + val scriptWit = P2WSHWitnessV0(spk) + val witSPK = P2WSHWitnessSPKV0(spk) + build(witSPK, signer, None, Some(scriptWit)) + } def p2wshOutputs: Gen[Seq[BitcoinUTXOSpendingInfo]] = Gen.choose(min, max).flatMap(n => Gen.listOfN(n, p2wshOutput)) @@ -230,6 +287,42 @@ sealed abstract class CreditingTxGen { def randoms: Gen[Seq[BitcoinUTXOSpendingInfo]] = Gen.choose(min, max).flatMap(n => Gen.listOfN(n, random)) + private def computeAllTrueConditionalPath( + spk: ScriptPubKey, + redeemScript: Option[ScriptPubKey], + scriptWitness: Option[ScriptWitness]): ConditionalPath = { + spk match { + case conditional: ConditionalScriptPubKey => + ConditionalPath.ConditionTrue( + computeAllTrueConditionalPath(conditional.trueSPK, None, None)) + case lockTimeScriptPubKey: LockTimeScriptPubKey => + computeAllTrueConditionalPath(lockTimeScriptPubKey.nestedScriptPubKey, + None, + None) + case _: RawScriptPubKey | _: P2WPKHWitnessSPKV0 => + ConditionalPath.NoConditionsLeft + case _: P2SHScriptPubKey => + redeemScript match { + case None => + throw new IllegalArgumentException( + "Expected redeem script for P2SH") + case Some(script) => + computeAllTrueConditionalPath(script, None, scriptWitness) + } + case _: P2WSHWitnessSPKV0 => + scriptWitness match { + case Some(witness: P2WSHWitnessV0) => + computeAllTrueConditionalPath(witness.redeemScript, None, None) + case _ => + throw new IllegalArgumentException( + "Expected P2WSHWitness for P2WSH") + } + case _: UnassignedWitnessScriptPubKey => + throw new IllegalArgumentException( + s"Unexpected unassigned witness SPK: $spk") + } + } + private def build( spk: ScriptPubKey, signers: Seq[Sign], @@ -249,7 +342,7 @@ sealed abstract class CreditingTxGen { redeemScript, scriptWitness, hashType, - ConditionalPath.NoConditionsLeft + computeAllTrueConditionalPath(spk, redeemScript, scriptWitness) ) } } diff --git a/testkit/src/main/scala/org/bitcoins/testkit/core/gen/ScriptGenerators.scala b/testkit/src/main/scala/org/bitcoins/testkit/core/gen/ScriptGenerators.scala index 4e066063f7..c2deeb3873 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/core/gen/ScriptGenerators.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/core/gen/ScriptGenerators.scala @@ -3,12 +3,14 @@ package org.bitcoins.testkit.core.gen import org.bitcoins.core.consensus.Consensus import org.bitcoins.core.crypto.{TransactionSignatureCreator, _} import org.bitcoins.core.currency.{CurrencyUnit, CurrencyUnits} -import org.bitcoins.core.number.UInt32 +import org.bitcoins.core.number.{UInt32, UInt64} import org.bitcoins.core.policy.Policy +import org.bitcoins.core.protocol.CompactSizeUInt import org.bitcoins.core.protocol.script.{P2SHScriptPubKey, _} import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.script.constant.ScriptNumber import org.bitcoins.core.script.crypto.HashType +import org.bitcoins.core.script.interpreter.ScriptInterpreter import org.bitcoins.core.util.BitcoinSLogger import org.bitcoins.core.wallet.signer.{MultiSigSigner, P2PKHSigner, P2PKSigner} import org.bitcoins.core.wallet.utxo.{ @@ -26,7 +28,15 @@ import scala.concurrent.duration.DurationInt //TODO: Need to provide generators for [[NonStandardScriptSignature]] and [[NonStandardScriptPubKey]] sealed abstract class ScriptGenerators extends BitcoinSLogger { val timeout = 5.seconds - private val defaultMaxDepth = 2 + val defaultMaxDepth: Int = 2 + + /** Since redeem scripts are pushed onto the stack, this function + * checks that the redeem script is not too large for a push operation. + */ + private[gen] def redeemScriptTooBig(redeemScript: ScriptPubKey): Boolean = { + redeemScript.compactSizeUInt.toInt + CompactSizeUInt(UInt64( + ScriptInterpreter.MAX_PUSH_SIZE)).bytes.length >= ScriptInterpreter.MAX_PUSH_SIZE + } def p2pkScriptSignature: Gen[P2PKScriptSignature] = for { @@ -220,6 +230,10 @@ sealed abstract class ScriptGenerators extends BitcoinSLogger { def p2shScriptPubKey: Gen[(P2SHScriptPubKey, Seq[ECPrivateKey])] = for { (randomScriptPubKey, privKeys) <- randomNonP2SHScriptPubKey + .suchThat { + case (spk, _) => + !redeemScriptTooBig(spk) + } p2sh = P2SHScriptPubKey(randomScriptPubKey) } yield (p2sh, privKeys) @@ -268,9 +282,14 @@ sealed abstract class ScriptGenerators extends BitcoinSLogger { } yield (P2WPKHWitnessSPKV0(privKey.publicKey), Seq(privKey)) def p2wshSPKV0: Gen[(P2WSHWitnessSPKV0, Seq[ECPrivateKey])] = - randomNonP2SHScriptPubKey.map { spk => - (P2WSHWitnessSPKV0(spk._1), spk._2) - } + randomNonP2SHScriptPubKey + .suchThat { + case (spk, _) => + !redeemScriptTooBig(spk) + } + .map { spk => + (P2WSHWitnessSPKV0(spk._1), spk._2) + } def witnessScriptPubKeyV0: Gen[(WitnessScriptPubKeyV0, Seq[ECPrivateKey])] = Gen.oneOf(p2wpkhSPKV0, p2wshSPKV0)