partial bitcoin#22154: Add OutputType::BECH32M and related wallet support for fetching bech32m addresses

includes:
- 754f134

Author: kwvg

src/coinjoin/client.cpp

@@ -1576,8 +1576,9 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
         // make our change address
         CScript scriptChange;
         CTxDestination dest;
+        bilingual_str error;
         ReserveDestination reserveDest(m_wallet.get());
-        bool success = reserveDest.GetReservedDestination(dest, true);
+        bool success = reserveDest.GetReservedDestination(dest, true, error);
         assert(success); // should never fail, as we just unlocked
         scriptChange = GetScriptForDestination(dest);
         reserveDest.KeepDestination();

src/coinjoin/util.cpp

@@ -29,7 +29,8 @@ inline unsigned int GetSizeOfCompactSizeDiff(uint64_t nSizePrev, uint64_t nSizeN
 CKeyHolder::CKeyHolder(CWallet* pwallet) :
     reserveDestination(pwallet)
 {
-    reserveDestination.GetReservedDestination(dest, false);
+    bilingual_str error;
+    reserveDestination.GetReservedDestination(dest, false, error);
 }
 
 void CKeyHolder::KeepKey()
@@ -100,8 +101,9 @@ CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBui
 {
     assert(pTxBuilder);
     CTxDestination txdest;
+    bilingual_str error;
     LOCK(wallet.cs_wallet);
-    dest.GetReservedDestination(txdest, false);
+    dest.GetReservedDestination(txdest, false, error);
     script = ::GetScriptForDestination(txdest);
 }
 

src/wallet/scriptpubkeyman.cpp

@@ -291,14 +291,16 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat
     return true;
 }
 
-bool LegacyScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool)
+bool LegacyScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error)
 {
     LOCK(cs_KeyStore);
     if (!CanGetAddresses(internal)) {
+        error = _("Error: Keypool ran out, please call keypoolrefill first");
         return false;
     }
 
     if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
+        error = _("Error: Keypool ran out, please call keypoolrefill first");
         return false;
     }
     // TODO: unify with bitcoin and use here GetDestinationForKey even if we have no type
@@ -1908,10 +1910,9 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
     return true;
 }
 
-bool DescriptorScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool)
+bool DescriptorScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error)
 {
     LOCK(cs_desc_man);
-    bilingual_str error;
     bool result = GetNewDestination(address, error);
     index = m_wallet_descriptor.next_index - 1;
     return result;

src/wallet/scriptpubkeyman.h

@@ -170,7 +170,7 @@ class ScriptPubKeyMan
     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(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { return false; }
+    virtual bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { return false; }
     virtual void KeepDestination(int64_t index) {}
     virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {}
 
@@ -324,7 +324,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
     bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
     bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override;
 
-    bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override;
+    bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override;
     void KeepDestination(int64_t index) override;
     void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override;
 
@@ -560,7 +560,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
     bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override;
 
-    bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override;
+    bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& 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

src/wallet/spend.cpp

@@ -742,8 +742,9 @@ static std::optional<CreatedTransactionResult> 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.");
+        bilingual_str dest_err;
+        if (!reservedest.GetReservedDestination(dest, true, dest_err)) {
+            error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + dest_err;
         }
         scriptChange = GetScriptForDestination(dest);
         // A valid destination implies a change script (and

src/wallet/test/coinjoin_tests.cpp

@@ -188,7 +188,7 @@ class CTransactionBuilderTestSetup : public TestChain100Setup
         coinControl.m_feerate = CFeeRate(1000);
         {
             LOCK(wallet->cs_wallet);
-            BOOST_CHECK(reserveDest.GetReservedDestination(tallyItem.txdest, false));
+            BOOST_CHECK(reserveDest.GetReservedDestination(tallyItem.txdest, false, strError));
         }
         for (CAmount nAmount : vecAmounts) {
             CTransactionRef tx;

src/wallet/wallet.cpp

@@ -2356,7 +2356,7 @@ bool CWallet::GetNewDestination(const std::string label, CTxDestination& dest, b
         spk_man->TopUp();
         result = spk_man->GetNewDestination(dest, error);
     } else {
-        error = strprintf(_("Error: No addresses available."));
+        error = _("Error: No addresses available.");
     }
     if (result) {
         SetAddressBook(dest, label, "receive");
@@ -2365,14 +2365,13 @@ bool CWallet::GetNewDestination(const std::string label, CTxDestination& dest, b
     return result;
 }
 
-bool CWallet::GetNewChangeDestination(CTxDestination& dest,  bilingual_str& error)
+bool CWallet::GetNewChangeDestination(CTxDestination& dest, bilingual_str& error)
 {
     LOCK(cs_wallet);
     error.clear();
 
     ReserveDestination reservedest(this);
-    if (!reservedest.GetReservedDestination(dest, true)) {
-        error = _("Error: Keypool ran out, please call keypoolrefill first");
+    if (!reservedest.GetReservedDestination(dest, true, error)) {
         return false;
     }
 
@@ -2446,10 +2445,11 @@ std::set<std::string> CWallet::ListAddrBookLabels(const std::string& purpose) co
     return label_set;
 }
 
-bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool fInternalIn)
+bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool fInternalIn, bilingual_str& error)
 {
     m_spk_man = pwallet->GetScriptPubKeyMan(fInternalIn);
     if (!m_spk_man) {
+        error = _("Error: No addresses available.");
         return false;
     }
 
@@ -2459,7 +2459,7 @@ bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool fInte
 
         CKeyPool keypool;
         int64_t index;
-        if (!m_spk_man->GetReservedDestination(fInternalIn, address, index, keypool)) {
+        if (!m_spk_man->GetReservedDestination(fInternalIn, address, index, keypool, error)) {
             return false;
         }
         nIndex = index;

src/wallet/wallet.h

@@ -203,7 +203,7 @@ class ReserveDestination
     }
 
     //! Reserve an address
-    bool GetReservedDestination(CTxDestination& pubkey, bool internal);
+    bool GetReservedDestination(CTxDestination& pubkey, bool internal, bilingual_str& error);
     //! Return reserved address
     void ReturnDestination();
     //! Keep the address. Do not return its key to the keypool when this object goes out of scope

test/functional/rpc_fundrawtransaction.py

@@ -583,7 +583,7 @@ def test_locked_wallet(self):
         # creating the key must be impossible because the wallet is locked
         outputs = {self.nodes[0].getnewaddress():value - Decimal("0.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)

test/functional/wallet_keypool_hd.py

@@ -197,7 +197,7 @@ def run_test(self):
 
         # 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.000010})
+        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.000010})
 
         # 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.000010})