From ec764fd951c5b981619c05ec622d6183397e10dc Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 17 Aug 2025 23:45:09 +0000 Subject: [PATCH 01/20] partial bitcoin#22154: Add OutputType::BECH32M and related wallet support for fetching bech32m addresses includes: - 754f134a50cc56cdf0baf996d909c992770fcc97 --- src/coinjoin/client.cpp | 3 ++- src/coinjoin/util.cpp | 6 ++++-- src/wallet/scriptpubkeyman.cpp | 7 ++++--- src/wallet/scriptpubkeyman.h | 6 +++--- src/wallet/spend.cpp | 5 +++-- src/wallet/test/coinjoin_tests.cpp | 2 +- src/wallet/wallet.cpp | 12 ++++++------ src/wallet/wallet.h | 2 +- test/functional/rpc_fundrawtransaction.py | 2 +- test/functional/wallet_keypool_hd.py | 2 +- 10 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 35593152c8b2..8e38cb9122b2 100644 --- a/src/coinjoin/client.cpp +++ b/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(); diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index 7b777c0311b8..e5b2cc629ec2 100644 --- a/src/coinjoin/util.cpp +++ b/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); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d8ef16cab882..5efca7c5f896 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/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; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 6b936e833e2c..31b45e283058 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/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 diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index fe2bb67acab8..ddf8d2bea813 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -742,8 +742,9 @@ static std::optional 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 diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index aaf29e9bfbce..80fc2dec49eb 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/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; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fd67bfb27f9f..653b2aed5bca 100644 --- a/src/wallet/wallet.cpp +++ b/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 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; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 00fc23ee1446..280749ba11f5 100644 --- a/src/wallet/wallet.h +++ b/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 diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 5cae04f7216a..d0f720a51191 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/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) diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py index 80cc5a02f219..8a6cc31321a7 100755 --- a/test/functional/wallet_keypool_hd.py +++ b/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}) From 9e23b112751554d004572fdf7c701c974c6c9a30 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 17 Aug 2025 22:20:20 +0000 Subject: [PATCH 02/20] merge bitcoin#25218: introduce generic 'Result' class and connect it to CreateTransaction and GetNewDestination excludes: - 7a45c33d1f8a758850cf8e7bd6ad508939ba5c0d continuation of 9f5845c501 from dash#6733 --- src/bench/wallet_loading.cpp | 7 +- src/coinjoin/util.cpp | 11 ++- src/interfaces/wallet.h | 7 +- src/qt/addresstablemodel.cpp | 14 ++- src/qt/walletmodel.cpp | 6 +- src/rpc/evo.cpp | 11 +-- src/test/util/wallet.cpp | 7 +- src/wallet/interfaces.cpp | 23 ++--- src/wallet/rpc/addresses.cpp | 18 ++-- src/wallet/rpc/spend.cpp | 12 +-- src/wallet/scriptpubkeyman.cpp | 32 +++--- src/wallet/scriptpubkeyman.h | 7 +- src/wallet/spend.cpp | 130 +++++++++++-------------- src/wallet/spend.h | 9 +- src/wallet/test/coinjoin_tests.cpp | 10 +- src/wallet/test/coinselector_tests.cpp | 8 +- src/wallet/test/fuzz/notifications.cpp | 11 +-- src/wallet/test/spend_tests.cpp | 14 +-- src/wallet/test/wallet_tests.cpp | 72 +++++++------- src/wallet/wallet.cpp | 30 +++--- src/wallet/wallet.h | 6 +- 21 files changed, 202 insertions(+), 243 deletions(-) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 7fdacc24529a..29e4e69ce545 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -47,12 +47,11 @@ static void BenchUnloadWallet(std::shared_ptr&& wallet) static void AddTx(CWallet& wallet) { - bilingual_str error; - CTxDestination dest; - wallet.GetNewDestination("", dest, error); + const auto& dest = wallet.GetNewDestination(""); + assert(dest.HasRes()); CMutableTransaction mtx; - mtx.vout.push_back({COIN, GetScriptForDestination(dest)}); + mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())}); mtx.vin.push_back(CTxIn()); wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index e5b2cc629ec2..408c6e8a425a 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -282,12 +282,13 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult) CTransactionRef tx; { LOCK2(m_wallet.cs_wallet, ::cs_main); - FeeCalculation fee_calc_out; - if (auto txr = wallet::CreateTransaction(m_wallet, vecSend, nChangePosRet, strResult, coinControl, fee_calc_out)) { - tx = txr->tx; - nFeeRet = txr->fee; - nChangePosRet = txr->change_pos; + auto ret = wallet::CreateTransaction(m_wallet, vecSend, nChangePosRet, coinControl); + if (ret) { + tx = ret.GetObj().tx; + nFeeRet = ret.GetObj().fee; + nChangePosRet = ret.GetObj().change_pos; } else { + strResult = ret.GetError(); return false; } } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 85e054b3cde2..3a51c4264ad8 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -112,7 +112,7 @@ class Wallet virtual std::string getWalletName() = 0; // Get a new address. - virtual bool getNewDestination(const std::string label, CTxDestination& dest) = 0; + virtual BResult getNewDestination(const std::string label) = 0; //! Get public key. virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0; @@ -167,12 +167,11 @@ class Wallet virtual std::vector listProTxCoins() = 0; //! Create transaction. - virtual CTransactionRef createTransaction(const std::vector& recipients, + virtual BResult createTransaction(const std::vector& recipients, const wallet::CCoinControl& coin_control, bool sign, int& change_pos, - CAmount& fee, - bilingual_str& fail_reason) = 0; + CAmount& fee) = 0; //! Commit transaction. virtual void commitTransaction(CTransactionRef tx, diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 4a09af9fff5b..3b91c01bf60f 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -366,23 +366,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con else if(type == Receive) { // Generate a new address to associate with given label - CTxDestination dest; - if(!walletModel->wallet().getNewDestination(strLabel, dest)) - { + auto op_dest = walletModel->wallet().getNewDestination(strLabel); + if (!op_dest) { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); - if(!ctx.isValid()) - { + if (!ctx.isValid()) { // Unlock wallet failed or was cancelled editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - if(!walletModel->wallet().getNewDestination(strLabel, dest)) - { + op_dest = walletModel->wallet().getNewDestination(strLabel); + if (!op_dest) { editStatus = KEY_GENERATION_FAILURE; return QString(); } } - strAddress = EncodeDestination(dest); + strAddress = EncodeDestination(op_dest.GetObj()); } else { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index a2c907c181b7..633fa9c95a99 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -251,11 +251,11 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact } CAmount nFeeRequired = 0; - bilingual_str error; int nChangePosRet = -1; auto& newTx = transaction.getWtx(); - newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error); + const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired); + newTx = res ? res.GetObj() : nullptr; transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && newTx) transaction.reassignAmounts(nChangePosRet); @@ -266,7 +266,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact { return SendCoinsReturn(AmountWithFeeExceedsBalance); } - Q_EMIT message(tr("Send Coins"), QString::fromStdString(error.translated), + Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated), CClientUIInterface::MSG_ERROR); return TransactionCreationFailed; } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 87c141b56790..17de56507eb7 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -281,15 +281,12 @@ static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const Specia throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("No funds at specified address %s", EncodeDestination(fundDest))); } - bilingual_str strFailReason; - FeeCalculation fee_calc_out; - auto txr = CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, strFailReason, coinControl, fee_calc_out, - true, tx.vExtraPayload.size()); - if (!txr) { - throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason.original); + auto res = CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, coinControl, /*sign=*/true, tx.vExtraPayload.size()); + if (!res) { + throw JSONRPCError(RPC_INTERNAL_ERROR, res.GetError().original); } - CTransactionRef newTx = txr->tx; + const CTransactionRef& newTx = res.GetObj().tx; tx.vin = newTx->vin; tx.vout = newTx->vout; diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index 190abea0c3c0..aad3538017f9 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -24,11 +24,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq #ifdef ENABLE_WALLET std::string getnewaddress(CWallet& w) { - CTxDestination dest; - bilingual_str error; - if (!w.GetNewDestination("", dest, error)) assert(false); + auto op_dest = w.GetNewDestination(""); + assert(op_dest.HasRes()); - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); } // void importaddress(CWallet& wallet, const std::string& address) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index e1a3970cd738..3724631b576b 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -175,11 +175,10 @@ class WalletImpl : public Wallet } int64_t getKeysLeftSinceAutoBackup() override { return m_wallet->nKeysLeftSinceAutoBackup; } std::string getWalletName() override { return m_wallet->GetName(); } - bool getNewDestination(const std::string label, CTxDestination& dest) override + BResult getNewDestination(const std::string label) override { LOCK(m_wallet->cs_wallet); - bilingual_str error; - return m_wallet->GetNewDestination(label, dest, error); + return m_wallet->GetNewDestination(label); } bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override { @@ -288,22 +287,20 @@ class WalletImpl : public Wallet LOCK(m_wallet->cs_wallet); return m_wallet->ListProTxCoins(); } - CTransactionRef createTransaction(const std::vector& recipients, + BResult createTransaction(const std::vector& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, - CAmount& fee, - bilingual_str& fail_reason) override + CAmount& fee) override { LOCK(m_wallet->cs_wallet); - FeeCalculation fee_calc_out; - std::optional txr = CreateTransaction(*m_wallet, recipients, change_pos, - fail_reason, coin_control, fee_calc_out, sign); - if (!txr) return {}; - fee = txr->fee; - change_pos = txr->change_pos; + const auto& res = CreateTransaction(*m_wallet, recipients, change_pos, coin_control, sign); + if (!res) return res.GetError(); + const auto& txr = res.GetObj(); + fee = txr.fee; + change_pos = txr.change_pos; - return txr->tx; + return txr.tx; } void commitTransaction(CTransactionRef tx, WalletValueMap value_map, diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index a23ba9e7c0cc..9fde4a32f07b 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -46,12 +46,11 @@ RPCHelpMan getnewaddress() if (!request.params[0].isNull()) label = LabelFromValue(request.params[0]); - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewDestination(label, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewDestination(label); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } @@ -80,12 +79,11 @@ RPCHelpMan getrawchangeaddress() throw JSONRPCError(RPC_WALLET_ERROR, "Error: This wallet has no available keys"); } - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewChangeDestination(dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewChangeDestination(); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index ef1012689961..ac5ce06e41ce 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -65,18 +65,16 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto } // Send - bilingual_str error; - FeeCalculation fee_calc_out; - std::optional txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true); - if (!txr) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); + auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, /*sign=*/true); + if (!res) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original); } - CTransactionRef tx = txr->tx; + const CTransactionRef& tx = res.GetObj().tx; wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); entry.pushKV("txid", tx->GetHash().GetHex()); - entry.pushKV("fee_reason", StringForFeeReason(fee_calc_out.reason)); + entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason)); return entry; } return tx->GetHash().GetHex(); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 5efca7c5f896..d16dbd6fd46b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -17,20 +17,17 @@ #include namespace wallet { -bool LegacyScriptPubKeyMan::GetNewDestination(CTxDestination& dest, bilingual_str& error) +BResult LegacyScriptPubKeyMan::GetNewDestination() { LOCK(cs_KeyStore); - error.clear(); // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, false)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } //LearnRelatedScripts(new_key); - dest = PKHash(new_key); - return true; + return CTxDestination(PKHash(new_key)); } typedef std::vector valtype; @@ -1781,12 +1778,11 @@ bool LegacyScriptPubKeyMan::GetHDChain(CHDChain& hdChainRet) const return !m_hd_chain.IsNull(); } -bool DescriptorScriptPubKeyMan::GetNewDestination(CTxDestination& dest, bilingual_str& error) +BResult DescriptorScriptPubKeyMan::GetNewDestination() { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later if (!CanGetAddresses()) { - error = _("No addresses available"); - return false; + return _("No addresses available"); } { LOCK(cs_desc_man); @@ -1799,15 +1795,14 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(CTxDestination& dest, bilingua std::vector scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } const OutputType type{OutputType::LEGACY}; + CTxDestination dest; std::optional out_script_type = m_wallet_descriptor.descriptor->GetOutputType(); if (out_script_type && out_script_type == type) { ExtractDestination(scripts_temp[0], dest); @@ -1816,7 +1811,7 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(CTxDestination& dest, bilingua } m_wallet_descriptor.next_index++; WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); - return true; + return dest; } } @@ -1913,9 +1908,14 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle bool DescriptorScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { LOCK(cs_desc_man); - bool result = GetNewDestination(address, error); + auto op_dest = GetNewDestination(); index = m_wallet_descriptor.next_index - 1; - return result; + if (op_dest) { + address = op_dest.GetObj(); + } else { + error = op_dest.GetError(); + } + return op_dest.HasRes(); } void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 31b45e283058..812c803be52e 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -12,6 +12,7 @@ #include