diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 2f7729a901..44c3912544 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -295,19 +295,22 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat return true; } -bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) +bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { + error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated; return false; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); if (!CanGetAddresses(internal)) { + error = _("Error: Keypool ran out, please call keypoolrefill first").translated; return false; } if (!ReserveKeyFromKeyPool(index, keypool, internal)) { + error = _("Error: Keypool ran out, please call keypoolrefill first").translated; return false; } address = GetDestinationForKey(keypool.vchPubKey, type); @@ -1720,10 +1723,9 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle return true; } -bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) +bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) { LOCK(cs_desc_man); - std::string error; bool result = GetNewDestination(type, address, error); index = m_wallet_descriptor.next_index - 1; return result; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 3c6a29e5d1..b2ca354b0a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -181,7 +181,7 @@ public: virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } - virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { return false; } + virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) { return false; } virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} @@ -364,7 +364,7 @@ public: bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override; + bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override; void KeepDestination(int64_t index, const OutputType& type) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override; @@ -573,7 +573,7 @@ public: bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override; + bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override; // Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index c8ded4c51e..6a8df437ae 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -618,8 +618,9 @@ bool CWallet::CreateTransactionInternal( // 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)) { - error = _("Transaction needs a change address, but we can't generate it. Please call keypoolrefill first."); + std::string dest_err; + if (!reservedest.GetReservedDestination(dest, true, dest_err)) { + error = strprintf(_("Transaction needs a change address, but we can't generate it. %s"), dest_err); } scriptChange = GetScriptForDestination(dest); // A valid destination implies a change script (and diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0f69302697..c2586b60b8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2118,7 +2118,7 @@ bool CWallet::GetNewDestination(const OutputType type, const std::string label, spk_man->TopUp(); result = spk_man->GetNewDestination(type, dest, error); } else { - error = strprintf("Error: No %s addresses available.", FormatOutputType(type)); + error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)).translated; } if (result) { SetAddressBook(dest, label, "receive"); @@ -2133,8 +2133,7 @@ bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& des error.clear(); ReserveDestination reservedest(this, type); - if (!reservedest.GetReservedDestination(dest, true)) { - error = _("Error: Keypool ran out, please call keypoolrefill first").translated; + if (!reservedest.GetReservedDestination(dest, true, error)) { return false; } @@ -2181,10 +2180,11 @@ std::set CWallet::GetLabelAddresses(const std::string& label) co return result; } -bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal) +bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, std::string& error) { m_spk_man = pwallet->GetScriptPubKeyMan(type, internal); if (!m_spk_man) { + error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)).translated; return false; } @@ -2194,7 +2194,7 @@ bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool inter m_spk_man->TopUp(); CKeyPool keypool; - if (!m_spk_man->GetReservedDestination(type, internal, address, nIndex, keypool)) { + if (!m_spk_man->GetReservedDestination(type, internal, address, nIndex, keypool, error)) { return false; } fInternal = keypool.fInternal; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 66f39edb4d..b63938c5f1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -181,7 +181,7 @@ public: } //! Reserve an address - bool GetReservedDestination(CTxDestination& pubkey, bool internal); + bool GetReservedDestination(CTxDestination& pubkey, bool internal, std::string& error); //! Return reserved address void ReturnDestination(); //! Keep the address. Do not return it's key to the keypool when this object goes out of scope diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 4b07a32c54..fa98c44152 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -551,7 +551,7 @@ class RawTransactionsTest(BitcoinTestFramework): # creating the key must be impossible because the wallet is locked 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) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", self.nodes[1].fundrawtransaction, rawtx) # Refill the keypool. self.nodes[1].walletpassphrase("test", 100) diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index b05fedcfc7..9b97d08424 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -374,10 +374,10 @@ class AddressTypeTest(BitcoinTestFramework): self.test_address(4, self.nodes[4].getrawchangeaddress('bech32'), multisig=False, typ='bech32') if self.options.descriptors: - self.log.info("Descriptor wallets do not have bech32m addreses by default yet") + self.log.info("Descriptor wallets do not have bech32m addresses by default yet") # TODO: Remove this when they do assert_raises_rpc_error(-12, "Error: No bech32m addresses available", self.nodes[0].getnewaddress, "", "bech32m") - assert_raises_rpc_error(-12, "Error: Keypool ran out, please call keypoolrefill first", self.nodes[0].getrawchangeaddress, "bech32m") + assert_raises_rpc_error(-12, "Error: No bech32m addresses available", self.nodes[0].getrawchangeaddress, "bech32m") else: self.log.info("Legacy wallets cannot make bech32m addresses") assert_raises_rpc_error(-8, "Legacy wallets cannot provide bech32m addresses", self.nodes[0].getnewaddress, "", "bech32m") diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 3fe6adeebc..28bfc9116f 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -161,7 +161,7 @@ class KeyPoolTest(BitcoinTestFramework): # 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}) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", 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})