merge bitcoin#21166: Introduce DeferredSignatureChecker and have SignatureExtractorClass subclass it

Author: kwvg

src/script/interpreter.h

@@ -189,6 +189,28 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker
 using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>;
 using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CMutableTransaction>;
 
+class DeferringSignatureChecker : public BaseSignatureChecker
+{
+protected:
+    BaseSignatureChecker& m_checker;
+
+public:
+    DeferringSignatureChecker(BaseSignatureChecker& checker) : m_checker(checker) {}
+
+    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
+    {
+        return m_checker.CheckSig(scriptSig, vchPubKey, scriptCode, sigversion);
+    }
+    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, ScriptError* error = nullptr);
 bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr);
 

src/script/sign.cpp

@@ -218,17 +218,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 CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
     {
-        if (checker.CheckSig(scriptSig, vchPubKey, scriptCode, sigversion)) {
+        if (m_checker.CheckSig(scriptSig, vchPubKey, scriptCode, sigversion)) {
             CPubKey pubkey(vchPubKey);
             sigdata.signatures.emplace(pubkey.GetID(), SigPair(pubkey, scriptSig));
             return true;

test/functional/rpc_signrawtransaction.py

@@ -4,12 +4,12 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test transaction signing using the signrawtransaction* RPCs."""
 
+from test_framework.address import script_to_p2sh
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (
-    assert_equal,
-    assert_raises_rpc_error,
-)
+from test_framework.util import assert_equal, assert_raises_rpc_error, find_vout_for_address
+from test_framework.script import CScript, OP_CHECKSEQUENCEVERIFY, OP_CHECKLOCKTIMEVERIFY, OP_DROP, OP_TRUE
 
+from decimal import Decimal, getcontext
 
 class SignRawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -153,12 +153,78 @@ def OP_1NEGATE_test(self):
         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.generate(self.nodes[0], 500)
+
+        # Create a P2SH script with CSV
+        script = CScript([1, OP_CHECKSEQUENCEVERIFY, OP_DROP, OP_TRUE])
+        address = script_to_p2sh(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.generate(self.nodes[0], 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()
+        )
+
+        # Sign and send the transaction
+        self.nodes[0].importaddress(script.hex(), p2sh=True)
+        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.generate(self.nodes[0], 1500)
+
+        # Create a P2SH script with CLTV
+        script = CScript([1000, OP_CHECKLOCKTIMEVERIFY, OP_DROP, OP_TRUE])
+        address = script_to_p2sh(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.generate(self.nodes[0], 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()
+        )
+
+        # Sign and send the transaction
+        self.nodes[0].importaddress(script.hex(), p2sh=True)
+        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()
         self.OP_1NEGATE_test()
         self.test_with_lock_outputs()
         self.test_fully_signed_tx()
+        # Descriptor wallets enforce stricter separation between wallets with and without private
+        # key while these tests require spending P2SH and non-P2SH inputs, which cannot reside in
+        # the same wallet, skip.
+        if not self.options.descriptors:
+            self.test_signing_with_csv()
+            self.test_signing_with_cltv()
 
 
 if __name__ == '__main__':