diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index abbe79c720..f5c3b3ac0c 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1027,15 +1027,16 @@ RPCHelpMan signrawtransactionwithwallet() const std::shared_ptr pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; + LOCK(pwallet->cs_wallet); + EnsureWalletIsUnlocked(*pwallet); + + int nHashType = ParseSighashString(request.params[2]); + CMutableTransaction mtx; if (!DecodeHexTx(mtx, request.params[0].get_str())) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input."); } - // Sign the transaction - LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(*pwallet); - // Fetch previous transactions (inputs): std::map coins; for (const CTxIn& txin : mtx.vin) { @@ -1046,14 +1047,13 @@ RPCHelpMan signrawtransactionwithwallet() // Parse the prevtxs array ParsePrevouts(request.params[1], nullptr, coins); - int nHashType = ParseSighashString(request.params[2]); - // Script verification errors std::map input_errors; bool immature_pegin = ValidateTransactionPeginInputs(mtx, pwallet->chain().getTip(), input_errors); bool complete = pwallet->SignTransaction(mtx, coins, nHashType, input_errors); UniValue result(UniValue::VOBJ); + // Sign the transaction SignTransactionResultToJSON(mtx, complete, coins, input_errors, immature_pegin, result); return result; }, diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 747a7d24ee..12e68cfc9d 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -402,7 +402,7 @@ class PSBTTest(BitcoinTestFramework): 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}) @@ -1393,6 +1393,9 @@ class PSBTTest(BitcoinTestFramework): # 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") + # ELEMENTS FIXME # self.log.info("Test decoding PSBT with per-input preimage types") # # note that the decodepsbt RPC doesn't check whether preimages and hashes match @@ -1496,5 +1499,9 @@ class PSBTTest(BitcoinTestFramework): # 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() diff --git a/test/functional/rpc_signrawtransactionwithkey.py b/test/functional/rpc_signrawtransactionwithkey.py index da94cdac09..f5a39c4af4 100755 --- a/test/functional/rpc_signrawtransactionwithkey.py +++ b/test/functional/rpc_signrawtransactionwithkey.py @@ -13,6 +13,7 @@ from test_framework.address import ( 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 ( @@ -32,6 +33,15 @@ from decimal import ( 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): self.setup_clean_chain = True @@ -63,6 +73,11 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): 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. @@ -72,25 +87,10 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): 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") @@ -112,9 +112,7 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): # 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 @@ -137,11 +135,15 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): # 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")}, {"fee": Decimal("0.001")}]) 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.nodes[0].createwallet(wallet_name="default") self.nodes[0].set_deterministic_priv_key('2Mysp7FKKe52eoC2JmU46irt1dt58TpCvhQ', 'cTNbtVJmhx75RXomhYWSZAafuNNNKPd1cr2ZiUcAeukLNGrHWjvJ') @@ -149,6 +151,7 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): self.successful_signing_test() self.witness_script_test() + self.invalid_sighashtype_test() if __name__ == '__main__': diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index a47c3add80..dd379faea7 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -326,7 +326,7 @@ class WalletTest(BitcoinTestFramework): 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 @@ class WalletTest(BitcoinTestFramework): 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) diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index d270114d16..8bb8649245 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -145,7 +145,7 @@ class BumpFeeTest(BitcoinTestFramework): 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) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 1be8afe570..2536759bf6 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -891,7 +891,7 @@ class RawTransactionsTest(BitcoinTestFramework): 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) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 8c4196d6ff..f2c2bfc7bc 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -392,7 +392,7 @@ class WalletSendTest(BitcoinTestFramework): 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). diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index d2322ccc71..a4ea09f2ba 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -33,6 +33,10 @@ from decimal import ( getcontext, ) + +RAW_TX = '020000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff01c0a6b929010000001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000' + + class SignRawTransactionWithWalletTest(BitcoinTestFramework): def add_options(self, parser): self.add_wallet_options(parser) @@ -57,10 +61,12 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): 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 = '02000000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff0101230f4f5d4b7c6fa845806ee4f67713459e1b69e8e60fcee2e4940c7a0d5de1b2010000000129b9a6c0001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000' - - 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. @@ -389,6 +395,7 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): 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()