Merge #21166: Introduce DeferredSignatureChecker and have SignatureExtractorClass subclass it

a97a9298cea085858e1a65a5e9b20d7a9e0f7303 Test that signrawtx works when a signed CSV and CLTV inputs are present (Andrew Chow)
6965456c10c9c4025c71c5e24fa5b27b15e5933a Introduce DeferringSignatureChecker and inherit with SignatureExtractor (Andrew Chow)

Pull request description:

  Previously SignatureExtractorChecker took a MutableTransactionSignatureChecker and passed through function calls to that. However not all functions were implemented so not everything passed through as it should have. To solve this, SignatureExctractorChecker now implements all of those functions via a new class - DeferredSignatureChecker. DeferredSignatureChecker is introduced to allow for future signature checkers which use another SignatureChecker but need to be able to do somethings outside of just the signature checking.

  Fixes #21151

ACKs for top commit:
  sipa:
    utACK a97a9298cea085858e1a65a5e9b20d7a9e0f7303
  meshcollider:
    Code review ACK a97a9298cea085858e1a65a5e9b20d7a9e0f7303
  instagibbs:
    utACK a97a9298cea085858e1a65a5e9b20d7a9e0f7303

Tree-SHA512: bca784c75c2fc3fcb74e81f4e3ff516699e8debaa2db81e12843abdfe9cf265dac11db8619751cb9b3e9bbe779805d029fabe5f3cbca5e86bfd72de3664b0b94

src/script/interpreter.h

@@ -272,6 +272,34 @@ public:
 using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>;
 using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CMutableTransaction>;
 
+class DeferringSignatureChecker : public BaseSignatureChecker
+{
+protected:
+    BaseSignatureChecker& m_checker;
+
+public:
+    DeferringSignatureChecker(BaseSignatureChecker& checker) : m_checker(checker) {}
+
+    bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
+    {
+        return m_checker.CheckECDSASignature(scriptSig, vchPubKey, scriptCode, sigversion);
+    }
+
+    bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, const ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override
+    {
+        return m_checker.CheckSchnorrSignature(sig, pubkey, sigversion, execdata, serror);
+    }
+
+    bool CheckLockTime(const CScriptNum& nLockTime) const override
+    {
+        return m_checker.CheckLockTime(nLockTime);
+    }
+    bool CheckSequence(const CScriptNum& nSequence) const override
+    {
+        return m_checker.CheckSequence(nSequence);
+    }
+};
+
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* error = nullptr);
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr);
 bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness* witness, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror = nullptr);

src/script/sign.cpp

@@ -250,17 +250,17 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
 }
 
 namespace {
-class SignatureExtractorChecker final : public BaseSignatureChecker
+class SignatureExtractorChecker final : public DeferringSignatureChecker
 {
 private:
     SignatureData& sigdata;
-    BaseSignatureChecker& checker;
 
 public:
-    SignatureExtractorChecker(SignatureData& sigdata, BaseSignatureChecker& checker) : sigdata(sigdata), checker(checker) {}
+    SignatureExtractorChecker(SignatureData& sigdata, BaseSignatureChecker& checker) : DeferringSignatureChecker(checker), sigdata(sigdata) {}
+
     bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
     {
-        if (checker.CheckECDSASignature(scriptSig, vchPubKey, scriptCode, sigversion)) {
+        if (m_checker.CheckECDSASignature(scriptSig, vchPubKey, scriptCode, sigversion)) {
             CPubKey pubkey(vchPubKey);
             sigdata.signatures.emplace(pubkey.GetID(), SigPair(pubkey, scriptSig));
             return true;

test/functional/rpc_signrawtransaction.py

@@ -4,16 +4,17 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test transaction signing using the signrawtransaction* RPCs."""
 
-from test_framework.address import check_script, script_to_p2sh
+from test_framework.address import check_script, script_to_p2sh, script_to_p2wsh
 from test_framework.key import ECKey
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_raises_rpc_error, find_vout_for_address, hex_str_to_bytes
-from test_framework.messages import sha256
-from test_framework.script import CScript, OP_0, OP_CHECKSIG
+from test_framework.messages import sha256, CTransaction, CTxInWitness
+from test_framework.script import CScript, OP_0, OP_CHECKSIG, OP_CHECKSEQUENCEVERIFY, OP_CHECKLOCKTIMEVERIFY, OP_DROP, OP_TRUE
 from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_p2wsh_script, script_to_p2wsh_script
 from test_framework.wallet_util import bytes_to_wif
 
-from decimal import Decimal
+from decimal import Decimal, getcontext
+from io import BytesIO
 
 class SignRawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -238,6 +239,78 @@ class SignRawTransactionsTest(BitcoinTestFramework):
         txn = self.nodes[0].signrawtransactionwithwallet(hex_str, prev_txs)
         assert txn["complete"]
 
+    def test_signing_with_csv(self):
+        self.log.info("Test signing a transaction containing a fully signed CSV input")
+        self.nodes[0].walletpassphrase("password", 9999)
+        getcontext().prec = 8
+
+        # Make sure CSV is active
+        self.nodes[0].generate(500)
+
+        # Create a P2WSH script with CSV
+        script = CScript([1, OP_CHECKSEQUENCEVERIFY, OP_DROP])
+        address = script_to_p2wsh(script)
+
+        # Fund that address and make the spend
+        txid = self.nodes[0].sendtoaddress(address, 1)
+        vout = find_vout_for_address(self.nodes[0], txid, address)
+        self.nodes[0].generate(1)
+        utxo = self.nodes[0].listunspent()[0]
+        amt = Decimal(1) + utxo["amount"] - Decimal(0.00001)
+        tx = self.nodes[0].createrawtransaction(
+            [{"txid": txid, "vout": vout, "sequence": 1},{"txid": utxo["txid"], "vout": utxo["vout"]}],
+            [{self.nodes[0].getnewaddress(): amt}],
+            self.nodes[0].getblockcount()
+        )
+
+        # Set the witness script
+        ctx = CTransaction()
+        ctx.deserialize(BytesIO(hex_str_to_bytes(tx)))
+        ctx.wit.vtxinwit.append(CTxInWitness())
+        ctx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE]), script]
+        tx = ctx.serialize_with_witness().hex()
+
+        # Sign and send the transaction
+        signed = self.nodes[0].signrawtransactionwithwallet(tx)
+        assert_equal(signed["complete"], True)
+        self.nodes[0].sendrawtransaction(signed["hex"])
+
+    def test_signing_with_cltv(self):
+        self.log.info("Test signing a transaction containing a fully signed CLTV input")
+        self.nodes[0].walletpassphrase("password", 9999)
+        getcontext().prec = 8
+
+        # Make sure CSV is active
+        self.nodes[0].generate(1500)
+
+        # Create a P2WSH script with CLTV
+        script = CScript([1000, OP_CHECKLOCKTIMEVERIFY, OP_DROP])
+        address = script_to_p2wsh(script)
+
+        # Fund that address and make the spend
+        txid = self.nodes[0].sendtoaddress(address, 1)
+        vout = find_vout_for_address(self.nodes[0], txid, address)
+        self.nodes[0].generate(1)
+        utxo = self.nodes[0].listunspent()[0]
+        amt = Decimal(1) + utxo["amount"] - Decimal(0.00001)
+        tx = self.nodes[0].createrawtransaction(
+            [{"txid": txid, "vout": vout},{"txid": utxo["txid"], "vout": utxo["vout"]}],
+            [{self.nodes[0].getnewaddress(): amt}],
+            self.nodes[0].getblockcount()
+        )
+
+        # Set the witness script
+        ctx = CTransaction()
+        ctx.deserialize(BytesIO(hex_str_to_bytes(tx)))
+        ctx.wit.vtxinwit.append(CTxInWitness())
+        ctx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE]), script]
+        tx = ctx.serialize_with_witness().hex()
+
+        # Sign and send the transaction
+        signed = self.nodes[0].signrawtransactionwithwallet(tx)
+        assert_equal(signed["complete"], True)
+        self.nodes[0].sendrawtransaction(signed["hex"])
+
     def run_test(self):
         self.successful_signing_test()
         self.script_verification_error_test()
@@ -245,6 +318,8 @@ class SignRawTransactionsTest(BitcoinTestFramework):
         self.OP_1NEGATE_test()
         self.test_with_lock_outputs()
         self.test_fully_signed_tx()
+        self.test_signing_with_csv()
+        self.test_signing_with_cltv()
 
 
 if __name__ == '__main__':