From 279b93f9e0d1607daebd0448dc4737e9d927102d Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Mon, 26 Apr 2021 12:46:50 -0500 Subject: [PATCH] Rework P2SHScriptSignature.isStandardNonP2SH() (#2963) * Take ben's suggestion for parsing the entire tx hex * Add assertion that the secondSPK is NOT a P2SHScriptPubKey, revert the classification of P2SHScriptPubKey in P2SHScriptSignature.isStandardNonP2SH() * Revert conditionalSPK match logic * Fixed ConditionalScriptPubKeyTest.scala (#32) Co-authored-by: Nadav Kohen --- .../script/ConditionalScriptPubKeyTest.scala | 17 +++++++-- .../protocol/script/ScriptSignature.scala | 36 ++++--------------- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/core-test/src/test/scala/org/bitcoins/core/protocol/script/ConditionalScriptPubKeyTest.scala b/core-test/src/test/scala/org/bitcoins/core/protocol/script/ConditionalScriptPubKeyTest.scala index 57aad77e15..656c14ea76 100644 --- a/core-test/src/test/scala/org/bitcoins/core/protocol/script/ConditionalScriptPubKeyTest.scala +++ b/core-test/src/test/scala/org/bitcoins/core/protocol/script/ConditionalScriptPubKeyTest.scala @@ -1,5 +1,6 @@ package org.bitcoins.core.protocol.script +import org.bitcoins.core.protocol.transaction.Transaction import org.bitcoins.core.script.constant.ScriptConstant import org.bitcoins.testkitcore.util.BitcoinSJvmTest @@ -27,10 +28,22 @@ class ConditionalScriptPubKeyTest extends BitcoinSJvmTest { "632103184b16f5d6c01e2d6ded3f8a292e5b81608318ecb8e93aa3747bc88b8dbf256cac67a914f5862841f254a1483eab66909ae588e45d617c5e8768" val scriptPubKey = RawScriptPubKey.fromAsmHex(hex) - assert(scriptPubKey.isInstanceOf[IfConditionalScriptPubKey]) - + scriptPubKey match { + case conditional: IfConditionalScriptPubKey => + assert( + conditional.secondSPK.isInstanceOf[NonStandardScriptPubKey], + s"Although we have same op codes as p2sh spk, " + + s"this combination isn't used in a standalone output, rather a redeemScript" + ) + case x => fail(s"Incorrect type for the spk, got=$x") + } val constant = ScriptConstant.fromHex(hex) assert(P2SHScriptSignature.isRedeemScript(constant)) + + //we should be able to parse the tx that contains it with no problem + val txHex = + "010000000193983cb2f1f3d83615f10e1ca33f49c72250acbebd840793aaa8bc7a6b28b76b000000008848304502207ffb30631d837895ac1b415408b70a809090b25d4070198043299fe247f62d2602210089b52f0d243dea1e4313ef67723b0d0642ff77bb6ca05ea85296b2539c797f5c81513d632103184b16f5d6c01e2d6ded3f8a292e5b81608318ecb8e93aa3747bc88b8dbf256cac67a914f5862841f254a1483eab66909ae588e45d617c5e8768ffffffff01a0860100000000001976a914a07aa8415d34d0db44f220ff9522609641c0bfbf88ac00000000" + assert(Transaction.fromHexT(txHex).isSuccess) } } diff --git a/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptSignature.scala b/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptSignature.scala index 04e6eae4b0..d9cd244c60 100644 --- a/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptSignature.scala +++ b/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptSignature.scala @@ -6,6 +6,7 @@ import org.bitcoins.core.util._ import org.bitcoins.core.wallet.utxo.ConditionalPath import org.bitcoins.crypto.{ECDigitalSignature, ECPublicKey} +import scala.annotation.tailrec import scala.util.Try /** Created by chris on 12/26/15. @@ -265,50 +266,25 @@ object P2SHScriptSignature extends ScriptFactory[P2SHScriptSignature] { } } + @tailrec private def isStandardNonP2SH( spk: ScriptPubKey, isRecursiveCall: Boolean): Boolean = { spk match { case _: P2PKHScriptPubKey | _: MultiSignatureScriptPubKey | _: P2PKScriptPubKey | _: P2PKWithTimeoutScriptPubKey | - _: WitnessScriptPubKeyV0 | _: UnassignedWitnessScriptPubKey => - true - case _: P2SHScriptPubKey => + _: UnassignedWitnessScriptPubKey | _: WitnessScriptPubKeyV0 | + _: ConditionalScriptPubKey => // Conditional SPKs are not recursively checked true case EmptyScriptPubKey => isRecursiveCall // Fine if nested - case conditional: ConditionalScriptPubKey => - val first = - isStandardNonP2SH(conditional.firstSPK, isRecursiveCall = true) - - val second = - isStandardNonP2SH(conditional.secondSPK, isRecursiveCall = true) - - //we need to see if we have a p2sh scriptpubkey nested - //inside of the conditional spk. This can actually happen - //when you literally just want to use the script OP_HASH160 OP_EQUAL - //independent of the normal p2sh flow - //see: https://github.com/bitcoin-s/bitcoin-s/issues/2962 - (first, second) match { - case (true, true) => true - case (true, false) => - P2SHScriptPubKey.isValidAsm(conditional.secondSPK.asm) - case (false, true) => - P2SHScriptPubKey.isValidAsm(conditional.firstSPK.asm) - case (false, false) => - val isP2SHFirst = - P2SHScriptPubKey.isValidAsm(conditional.firstSPK.asm) - val isP2SHSecond = - P2SHScriptPubKey.isValidAsm(conditional.secondSPK.asm) - - isP2SHFirst && isP2SHSecond - } case locktime: LockTimeScriptPubKey => if (Try(locktime.locktime).isSuccess) { isStandardNonP2SH(locktime.nestedScriptPubKey, isRecursiveCall = true) } else { false } - case _: NonStandardScriptPubKey | _: WitnessCommitment => + case _: NonStandardScriptPubKey | _: WitnessCommitment | + _: P2SHScriptPubKey => false } }