From 5efc25f9638866941028454cfa9bae27f1519cb4 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Sat, 12 Oct 2019 12:27:56 +0200 Subject: [wallet] translate "Keypool ran out" message --- src/wallet/scriptpubkeyman.cpp | 2 +- src/wallet/wallet.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 4c9d88973e..1a9f0fe0c8 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -18,7 +18,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestinat // 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 index 4a38571dfc..b53c251131 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2638,7 +2638,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std bool ret = reservedest.GetReservedDestination(dest, true); if (!ret) { - strFailReason = "Keypool ran out, please call keypoolrefill first"; + strFailReason = _("Keypool ran out, please call keypoolrefill first").translated; return false; } @@ -3161,7 +3161,7 @@ bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& des 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; } -- cgit v1.2.3 From 709f8685ac37510aa145ac259753583c82280038 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 23 Oct 2019 15:11:25 +0200 Subject: [wallet] CreateTransaction: simplify change address check --- src/wallet/wallet.cpp | 11 ++--------- test/functional/rpc_fundrawtransaction.py | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b53c251131..8e20f48ae3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2630,18 +2630,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std // post-backup change. // Reserve a new key pair from key pool - if (!CanGetAddresses(true)) { - strFailReason = _("Can't generate a change-address key. No keys in the internal keypool and can't generate any keys.").translated; - return false; - } CTxDestination dest; - bool ret = reservedest.GetReservedDestination(dest, true); - if (!ret) - { - strFailReason = _("Keypool ran out, please call keypoolrefill first").translated; + if (!reservedest.GetReservedDestination(dest, true)) { + strFailReason = _("Can't generate a change-address key. Please call keypoolrefill first.").translated; return false; } - scriptChange = GetScriptForDestination(dest); } CTxOut change_prototype_txout(0, scriptChange); diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 6f1ae0d3ba..8a2dc7e916 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -504,7 +504,7 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = self.nodes[1].createrawtransaction(inputs, outputs) # 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) + assert_raises_rpc_error(-4, "Can't generate a change-address key. Please call keypoolrefill first.", self.nodes[1].fundrawtransaction, rawtx) # Refill the keypool. self.nodes[1].walletpassphrase("test", 100) -- cgit v1.2.3 From 92bcd70808b9cac56b184903aa6d37baf9641b37 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 23 Oct 2019 15:21:50 +0200 Subject: [wallet] allow transaction without change if keypool is empty --- src/wallet/wallet.cpp | 12 +++++-- test/functional/rpc_fundrawtransaction.py | 9 +++-- test/functional/wallet_keypool.py | 60 +++++++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8e20f48ae3..5b4781beaf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2629,13 +2629,14 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std // rediscover unknown transactions that were written with keys of ours to recover // post-backup change. - // Reserve a new key pair from key pool + // Reserve a new key pair from key pool. If it fails, provide a dummy + // destination in case we don't need change. CTxDestination dest; if (!reservedest.GetReservedDestination(dest, true)) { - strFailReason = _("Can't generate a change-address key. Please call keypoolrefill first.").translated; - return false; + strFailReason = _("Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.").translated; } scriptChange = GetScriptForDestination(dest); + assert(!dest.empty() || scriptChange.empty()); } CTxOut change_prototype_txout(0, scriptChange); coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout); @@ -2851,6 +2852,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std coin_selection_params.use_bnb = false; 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 diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 8a2dc7e916..03c5271058 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -500,11 +500,16 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[1].getnewaddress() self.nodes[1].getrawchangeaddress() inputs = [] - outputs = {self.nodes[0].getnewaddress():1.1} + outputs = {self.nodes[0].getnewaddress():1.09999500} 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, "Can't generate a change-address key. 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 index e3aeb61197..9e2f00e62f 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -5,6 +5,7 @@ """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 @@ -53,12 +54,12 @@ class KeyPoolTest(BitcoinTestFramework): assert_raises_rpc_error(-12, "Keypool ran out", nodes[0].getrawchangeaddress) # drain the external keys - addr.add(nodes[0].getnewaddress()) - addr.add(nodes[0].getnewaddress()) - addr.add(nodes[0].getnewaddress()) - addr.add(nodes[0].getnewaddress()) - addr.add(nodes[0].getnewaddress()) - addr.add(nodes[0].getnewaddress()) + addr.add(nodes[0].getnewaddress(address_type="bech32")) + addr.add(nodes[0].getnewaddress(address_type="bech32")) + addr.add(nodes[0].getnewaddress(address_type="bech32")) + addr.add(nodes[0].getnewaddress(address_type="bech32")) + addr.add(nodes[0].getnewaddress(address_type="bech32")) + addr.add(nodes[0].getnewaddress(address_type="bech32")) assert len(addr) == 6 # the next one should fail assert_raises_rpc_error(-12, "Error: Keypool ran out, please call keypoolrefill first", nodes[0].getnewaddress) @@ -82,5 +83,52 @@ class KeyPoolTest(BitcoinTestFramework): 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.00008900}], options={"feeRate": 0.00010}) + assert_equal("psbt" in res, True) + + # dust change should be removed + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008800}], 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.0008824}) + assert_equal("psbt" in res, True) + assert_equal(res["fee"], Decimal("0.00009706")) + + # 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() -- cgit v1.2.3