Merge bitcoin/bitcoin#28166: test, rpc: invalid sighashtype coverage

90c8f79 test: remove redundant test values (Jon Atack)
c3f2033 test: use common assert_signing_completed_successfully helper (Jon Atack)
647d95a test: add coverage for passing an invalid sighashtype (Jon Atack)

Pull request description:

  Add test coverage for passing an invalid sighashtype to RPCs signrawtransactionwithwallet, signrawtransactionwithkey, walletprocesspsbt, and descriptorprocesspsbt.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 90c8f79 🎥
  brunoerg:
    light crACK 90c8f79

Tree-SHA512: 3861658487edd0d9a377390acf3d43f45c3dd9e324894f0fdb8f5312b618301a55479b1f70c61daee0b20785e768ffde6fa5abe6af190b73c0d0e017f3976704

Author: fanquake

test/functional/rpc_psbt.py

@@ -327,7 +327,7 @@ def run_test(self):
                 assert_raises_rpc_error(-3, "Invalid amount",
                     self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: invalid_value, "add_inputs": True})
         # Test fee_rate values that cannot be represented in sat/vB.
-        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
             assert_raises_rpc_error(-3, "Invalid amount",
                 self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": invalid_value, "add_inputs": True})
 
@@ -883,6 +883,9 @@ def test_psbt_input_keys(psbt_input, keys):
             comb_psbt = self.nodes[0].combinepsbt([psbt, parsed_psbt.to_base64()])
             assert_equal(comb_psbt, psbt)
 
+        self.log.info("Test walletprocesspsbt raises if an invalid sighashtype is passed")
+        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all")
+
         self.log.info("Test decoding PSBT with per-input preimage types")
         # note that the decodepsbt RPC doesn't check whether preimages and hashes match
         hash_ripemd160, preimage_ripemd160 = random_bytes(20), random_bytes(50)
@@ -982,5 +985,9 @@ def test_psbt_input_keys(psbt_input, keys):
         rawtx = self.nodes[2].finalizepsbt(psbt)["hex"]
         self.nodes[2].sendrawtransaction(rawtx)
 
+        self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed")
+        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all")
+
+
 if __name__ == '__main__':
     PSBTTest().main()

test/functional/rpc_signrawtransactionwithkey.py

@@ -14,6 +14,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_raises_rpc_error,
     find_vout_for_address,
 )
 from test_framework.script_util import (
@@ -33,6 +34,14 @@
     Decimal,
 )
 
+INPUTS = [
+    # Valid pay-to-pubkey scripts
+    {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0,
+     'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'},
+    {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0,
+     'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'},
+]
+OUTPUTS = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1}
 
 class SignRawTransactionWithKeyTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -47,6 +56,11 @@ def send_to_address(self, addr, amount):
         txid = self.nodes[0].sendrawtransaction(self.nodes[0].signrawtransactionwithkey(rawtx, [self.nodes[0].get_deterministic_priv_key().key])["hex"], 0)
         return txid
 
+    def assert_signing_completed_successfully(self, signed_tx):
+        assert 'errors' not in signed_tx
+        assert 'complete' in signed_tx
+        assert_equal(signed_tx['complete'], True)
+
     def successful_signing_test(self):
         """Create and sign a valid raw transaction with one input.
 
@@ -56,25 +70,10 @@ def successful_signing_test(self):
         2) No script verification error occurred"""
         self.log.info("Test valid raw transaction with one input")
         privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA']
+        rawTx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS)
+        rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, INPUTS)
 
-        inputs = [
-            # Valid pay-to-pubkey scripts
-            {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0,
-             'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'},
-            {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0,
-             'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'},
-        ]
-
-        outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1}
-
-        rawTx = self.nodes[0].createrawtransaction(inputs, outputs)
-        rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, inputs)
-
-        # 1) The transaction has a complete set of signatures
-        assert rawTxSigned['complete']
-
-        # 2) No script verification error occurred
-        assert 'errors' not in rawTxSigned
+        self.assert_signing_completed_successfully(rawTxSigned)
 
     def witness_script_test(self):
         self.log.info("Test signing transaction to P2SH-P2WSH addresses without wallet")
@@ -95,9 +94,7 @@ def witness_script_test(self):
         # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys
         spending_tx = self.nodes[0].createrawtransaction([unspent_output], {getnewdestination()[2]: Decimal("49.998")})
         spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [unspent_output])
-        # Check the signing completed successfully
-        assert 'complete' in spending_tx_signed
-        assert_equal(spending_tx_signed['complete'], True)
+        self.assert_signing_completed_successfully(spending_tx_signed)
 
         # Now test with P2PKH and P2PK scripts as the witnessScript
         for tx_type in ['P2PKH', 'P2PK']:  # these tests are order-independent
@@ -120,14 +117,19 @@ def verify_txn_with_witness_script(self, tx_type):
         # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys
         spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {getnewdestination()[2]: Decimal("9.999")})
         spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}])
-        # Check the signing completed successfully
-        assert 'complete' in spending_tx_signed
-        assert_equal(spending_tx_signed['complete'], True)
+        self.assert_signing_completed_successfully(spending_tx_signed)
         self.nodes[0].sendrawtransaction(spending_tx_signed['hex'])
 
+    def invalid_sighashtype_test(self):
+        self.log.info("Test signing transaction with invalid sighashtype")
+        tx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS)
+        privkeys = [self.nodes[0].get_deterministic_priv_key().key]
+        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all")
+
     def run_test(self):
         self.successful_signing_test()
         self.witness_script_test()
+        self.invalid_sighashtype_test()
 
 
 if __name__ == '__main__':

test/functional/wallet_basic.py

@@ -328,7 +328,7 @@ def run_test(self):
         for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]:
             assert_raises_rpc_error(-3, msg, self.nodes[2].sendmany, amounts={address: 1.0}, fee_rate=invalid_value)
         # Test fee_rate values that cannot be represented in sat/vB.
-        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
             assert_raises_rpc_error(-3, msg, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=invalid_value)
         # Test fee_rate out of range (negative number).
         assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=-1)
@@ -523,7 +523,7 @@ def run_test(self):
             for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]:
                 assert_raises_rpc_error(-3, msg, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=invalid_value)
             # Test fee_rate values that cannot be represented in sat/vB.
-            for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+            for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
                 assert_raises_rpc_error(-3, msg, self.nodes[2].sendtoaddress, address=address, amount=10, fee_rate=invalid_value)
             # Test fee_rate out of range (negative number).
             assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=-1)

test/functional/wallet_bumpfee.py

@@ -141,7 +141,7 @@ def test_invalid_parameters(self, rbf_node, peer_node, dest_address):
         for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]:
             assert_raises_rpc_error(-3, msg, rbf_node.bumpfee, rbfid, fee_rate=invalid_value)
         # Test fee_rate values that cannot be represented in sat/vB.
-        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
             assert_raises_rpc_error(-3, msg, rbf_node.bumpfee, rbfid, fee_rate=invalid_value)
         # Test fee_rate out of range (negative number).
         assert_raises_rpc_error(-3, "Amount out of range", rbf_node.bumpfee, rbfid, fee_rate=-1)

test/functional/wallet_fundrawtransaction.py

@@ -832,7 +832,7 @@ def test_option_feerate(self):
             for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]:
                 assert_raises_rpc_error(-3, "Invalid amount", node.fundrawtransaction, rawtx, add_inputs=True, **{param: invalid_value})
         # Test fee_rate values that cannot be represented in sat/vB.
-        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
             assert_raises_rpc_error(-3, "Invalid amount",
                 node.fundrawtransaction, rawtx, fee_rate=invalid_value, add_inputs=True)
 

test/functional/wallet_send.py

@@ -387,7 +387,7 @@ def run_test(self):
             self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg))
             self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg))
         # Test fee_rate values that cannot be represented in sat/vB.
-        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]:
+        for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]:
             self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg))
             self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg))
         # Test fee_rate out of range (negative number).

test/functional/wallet_signrawtransactionwithwallet.py

@@ -33,6 +33,10 @@
     getcontext,
 )
 
+
+RAW_TX = '020000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff01c0a6b929010000001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000'
+
+
 class SignRawTransactionWithWalletTest(BitcoinTestFramework):
     def add_options(self, parser):
         self.add_wallet_options(parser)
@@ -47,10 +51,12 @@ def skip_test_if_missing_module(self):
     def test_with_lock_outputs(self):
         self.log.info("Test correct error reporting when trying to sign a locked output")
         self.nodes[0].encryptwallet("password")
+        assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].signrawtransactionwithwallet, RAW_TX)
+        self.nodes[0].walletpassphrase("password", 9999)
 
-        rawTx = '020000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff01c0a6b929010000001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000'
-
-        assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].signrawtransactionwithwallet, rawTx)
+    def test_with_invalid_sighashtype(self):
+        self.log.info("Test signrawtransactionwithwallet raises if an invalid sighashtype is passed")
+        assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all")
 
     def script_verification_error_test(self):
         """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script.
@@ -299,6 +305,7 @@ def run_test(self):
         self.script_verification_error_test()
         self.OP_1NEGATE_test()
         self.test_with_lock_outputs()
+        self.test_with_invalid_sighashtype()
         self.test_fully_signed_tx()
         self.test_signing_with_csv()
         self.test_signing_with_cltv()