diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -22,7 +22,8 @@ // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - error = "Error: Keypool ran out, please call keypoolrefill first"; + error = _("Error: Keypool ran out, please call keypoolrefill first") + .translated; return false; } LearnRelatedScripts(new_key, type); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3069,20 +3069,16 @@ // for and rediscover unknown transactions that were written // with keys of ours to recover post-backup change. - // Reserve a new key pair from key pool - if (!CanGetAddresses(true)) { - error = _("Can't generate a change-address key. No keys in the " - "internal keypool and can't generate any keys."); - return false; - } + // Reserve a new key pair from key pool. If it fails, provide a + // dummy destination in case we don't need change. CTxDestination dest; - bool ret = reservedest.GetReservedDestination(dest, true); - if (!ret) { - error = _("Keypool ran out, please call keypoolrefill first"); - return false; + if (!reservedest.GetReservedDestination(dest, true)) { + error = _("Transaction needs a change address, but we can't " + "generate it. Please call keypoolrefill first."); } scriptChange = GetScriptForDestination(dest); + assert(!dest.empty() || scriptChange.empty()); } CTxOut change_prototype_txout(Amount::zero(), scriptChange); coin_selection_params.change_output_size = @@ -3321,6 +3317,12 @@ continue; } + // Give up if change keypool ran out and we failed to find a solution + // without change: + if (scriptChange.empty() && nChangePosInOut != -1) { + return false; + } + // Shuffle selected coins and fill in final vin txNew.vin.clear(); std::vector selected_coins(setCoins.begin(), @@ -3655,7 +3657,8 @@ ReserveDestination reservedest(this, type); if (!reservedest.GetReservedDestination(dest, true)) { - error = "Error: Keypool ran out, please call keypoolrefill first"; + error = _("Error: Keypool ran out, please call keypoolrefill first") + .translated; return false; } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -545,12 +545,21 @@ self.nodes[1].getnewaddress() self.nodes[1].getrawchangeaddress() inputs = [] - outputs = {self.nodes[0].getnewaddress(): 1.1} - rawTx = self.nodes[1].createrawtransaction(inputs, outputs) + outputs = {self.nodes[0].getnewaddress(): 1.09999700} + rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + # fund a transaction that does not require a new key for the change + # output + self.nodes[1].fundrawtransaction(rawtx) + # fund a transaction that requires a new key for the change output # creating the key must be impossible because the wallet is locked - assert_raises_rpc_error(-4, "Keypool ran out, please call keypoolrefill first", - self.nodes[1].fundrawtransaction, rawTx) + outputs = {self.nodes[0].getnewaddress(): 1.1} + rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + assert_raises_rpc_error( + -4, + "Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.", + self.nodes[1].fundrawtransaction, + rawtx) # Refill the keypool. self.nodes[1].walletpassphrase("test", 100) diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet keypool and interaction with wallet encryption/locking.""" import time +from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error @@ -89,6 +90,81 @@ assert_equal(wi['keypoolsize_hd_internal'], 100) assert_equal(wi['keypoolsize'], 100) + # create a blank wallet + nodes[0].createwallet(wallet_name='w2', blank=True) + w2 = nodes[0].get_wallet_rpc('w2') + + # refer to initial wallet as w1 + w1 = nodes[0].get_wallet_rpc('') + + # import private key and fund it + address = addr.pop() + privkey = w1.dumpprivkey(address) + res = w2.importmulti( + [{'scriptPubKey': {'address': address}, 'keys': [privkey], 'timestamp': 'now'}]) + assert_equal(res[0]['success'], True) + w1.walletpassphrase('test', 100) + + res = w1.sendtoaddress(address=address, amount=0.00010000) + nodes[0].generate(1) + destination = addr.pop() + + # Using a fee rate (10 sat / byte) well above the minimum relay rate + # creating a 5,000 sat transaction with change should not be possible + assert_raises_rpc_error(-4, + "Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.", + w2.walletcreatefundedpsbt, + inputs=[], + outputs=[{addr.pop(): 0.00005000}], + options={"subtractFeeFromOutputs": [0], + "feeRate": 0.00010}) + + # creating a 10,000 sat transaction without change, with a manual + # input, should still be possible + res = w2.walletcreatefundedpsbt( + inputs=w2.listunspent(), + outputs=[{destination: 0.00010000}], + options={"subtractFeeFromOutputs": [0], "feeRate": 0.00010}) + assert_equal("psbt" in res, True) + + # creating a 10,000 sat transaction without change should still be + # possible + res = w2.walletcreatefundedpsbt( + inputs=[], + outputs=[{destination: 0.00010000}], + options={"subtractFeeFromOutputs": [0], "feeRate": 0.00010}) + assert_equal("psbt" in res, True) + # should work without subtractFeeFromOutputs if the exact fee is + # subtracted from the amount + res = w2.walletcreatefundedpsbt(inputs=[], + outputs=[{destination: 0.00008000}], + options={"feeRate": 0.00010}) + assert_equal("psbt" in res, True) + + # dust change should be removed + res = w2.walletcreatefundedpsbt(inputs=[], + outputs=[{destination: 0.00007900}], + options={"feeRate": 0.00010}) + assert_equal("psbt" in res, True) + + # create a transaction without change at the maximum fee rate, such + # that the output is still spendable: + res = w2.walletcreatefundedpsbt( + inputs=[], + outputs=[{destination: 0.00010000}], + options={"subtractFeeFromOutputs": [0], "feeRate": 0.0004949}) + assert_equal("psbt" in res, True) + assert_equal(res["fee"], Decimal("0.00009453")) + + # creating a 10,000 sat transaction with a manual change address should + # be possible + res = w2.walletcreatefundedpsbt(inputs=[], + outputs=[{destination: 0.00010000}], + options={"subtractFeeFromOutputs": [0], + "feeRate": 0.00010, + "changeAddress": addr.pop()}) + assert_equal("psbt" in res, True) + if __name__ == '__main__': KeyPoolTest().main()