backport: merge bitcoin#25218, #25721, #25656, #25616, #26005, #24855, #18554, #17204, #20562, #21166, #25044, #25364, #25525, #25512, #24678, #26747, partial bitcoin#22154, #24584 (wallet backports: part 8) #6823
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,7 +78,8 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const | |
| "-flushwallet", | ||
| "-privdb", | ||
| "-walletrejectlongchains", | ||
| "-walletcrosschain", | ||
| "-unsafesqlitesync", | ||
| }); | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,7 +112,7 @@ class Wallet | |
| virtual std::string getWalletName() = 0; | ||
|
|
||
| // Get a new address. | ||
| virtual util::Result<CTxDestination> 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<COutPoint> listProTxCoins() = 0; | ||
|
|
||
| //! Create transaction. | ||
| virtual util::Result<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients, | ||
| const wallet::CCoinControl& coin_control, | ||
| bool sign, | ||
| int& change_pos, | ||
| CAmount& fee) = 0; | ||
|
|
||
| //! Commit transaction. | ||
| virtual void commitTransaction(CTransactionRef tx, | ||
|
|
@@ -348,35 +347,35 @@ class Wallet | |
| class WalletLoader : public ChainClient | ||
| { | ||
| public: | ||
| //! Register non-core wallet RPCs | ||
| virtual void registerOtherRpcs(const Span<const CRPCCommand>& commands) = 0; | ||
|
|
||
| //! Create new wallet. | ||
| virtual util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) = 0; | ||
|
|
||
| //! Load existing wallet. | ||
| virtual util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) = 0; | ||
|
|
||
| //! Return default wallet directory. | ||
| virtual std::string getWalletDir() = 0; | ||
|
|
||
| //! Restore backup wallet | ||
| virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0; | ||
|
|
||
| //! Return available wallets in wallet directory. | ||
| virtual std::vector<std::string> listWalletDir() = 0; | ||
|
|
||
| //! Return interfaces for accessing wallets (if any). | ||
| virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; | ||
|
|
||
| //! Register handler for load wallet messages. This callback is triggered by | ||
| //! createWallet and loadWallet above, and also triggered when wallets are | ||
| //! loaded at startup or by RPC. | ||
| using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; | ||
| virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; | ||
|
|
||
| //! Return pointer to internal context, useful for testing. | ||
| virtual wallet::WalletContext* context() { return nullptr; } | ||
| }; | ||
|
Comment on lines
+350
to
379
There was a problem hiding this comment. 💡 Verification agent 🧩 Analysis chainWalletLoader methods migrated to util::Result — good, but verify call sites are updated The create/load/restore methods now return util::Result and drop the error out-param. This ripples through GUI/RPC. Please verify there are no remaining callers expecting the old signatures. Run this script to catch old-style usages (extra args/out-params): 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Searching for old getNewDestination usages (with extra out-params)..."
rg -nP --type=cpp -C2 '\b(getNewDestination|GetNewDestination)\s*\(\s*[^,()]+\s*,\s*[^)]' || true
echo
echo "Searching for old createTransaction usages (fail_reason out-param or boolean return)..."
# Look for calls passing a bilingual_str or expecting bool
rg -nP --type=cpp -C3 '\bcreateTransaction\s*\(' || true
rg -nP --type=cpp -C2 'bilingual_str\s+\w+\s*;.*\bcreateTransaction\s*\(' || true
echo
echo "Searching for WalletLoader old create/load/restore signatures..."
rg -nP --type=cpp -C2 '\.createWallet\s*\([^)]{60,}\)' || true
rg -nP --type=cpp -C2 '\.loadWallet\s*\([^)]{60,}\)' || true
rg -nP --type=cpp -C2 '\.restoreWallet\s*\([^)]{60,}\)' || trueLength of output: 2428 Fix outdated createTransaction usage in Qt WalletModel We confirmed there are no remaining old-style calls to • src/qt/walletmodel.cpp:257 const auto& res = m_wallet->createTransaction(
vecSend, coinControl, !wallet().privateKeysDisabled(), nChangePosRet, nFeeRequired);with a call matching the interface’s new signature (e.g. passing only 🤖 Prompt for AI Agents |
||
|
|
||
| //! Information about one wallet address. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| auto op_dest = walletModel->wallet().getNewDestination(strLabel); | ||
| if (!op_dest) { | ||
| WalletModel::UnlockContext ctx(walletModel->requestUnlock()); | ||
| if (!ctx.isValid()) { | ||
| // Unlock wallet failed or was cancelled | ||
| editStatus = WALLET_UNLOCK_FAILURE; | ||
| return QString(); | ||
| } | ||
| op_dest = walletModel->wallet().getNewDestination(strLabel); | ||
| if (!op_dest) { | ||
| editStatus = KEY_GENERATION_FAILURE; | ||
| return QString(); | ||
| } | ||
| } | ||
| strAddress = EncodeDestination(*op_dest); | ||
| } | ||
| else | ||
| { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -258,9 +258,13 @@ void CreateWalletActivity::createWallet() | |
| } | ||
|
|
||
| QTimer::singleShot(500ms, worker(), [this, name, flags] { | ||
| auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)}; | ||
|
|
||
| if (wallet) { | ||
| m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet)); | ||
| } else { | ||
| m_error_message = util::ErrorString(wallet); | ||
| } | ||
|
|
||
| QTimer::singleShot(500ms, this, &CreateWalletActivity::finish); | ||
| }); | ||
|
|
@@ -330,9 +334,13 @@ void OpenWalletActivity::open(const std::string& path) | |
| tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped())); | ||
|
|
||
| QTimer::singleShot(0, worker(), [this, path] { | ||
| auto wallet{node().walletLoader().loadWallet(path, m_warning_message)}; | ||
|
|
||
| if (wallet) { | ||
| m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet)); | ||
| } else { | ||
| m_error_message = util::ErrorString(wallet); | ||
| } | ||
|
|
||
| QTimer::singleShot(0, this, &OpenWalletActivity::finish); | ||
| }); | ||
|
|
@@ -375,8 +383,11 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri | |
| QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] { | ||
| auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)}; | ||
|
|
||
| if (wallet) { | ||
| m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet)); | ||
| } else { | ||
| m_error_message = util::ErrorString(wallet); | ||
| } | ||
|
|
||
| QTimer::singleShot(0, this, &RestoreWalletActivity::finish); | ||
| }); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -251,11 +251,11 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact | |
| } | ||
|
|
||
| CAmount nFeeRequired = 0; | ||
| int nChangePosRet = -1; | ||
|
|
||
| auto& newTx = transaction.getWtx(); | ||
| const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired); | ||
| newTx = res ? *res : 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(util::ErrorString(res).translated), | ||
| CClientUIInterface::MSG_ERROR); | ||
| return TransactionCreationFailed; | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,7 +270,7 @@ static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const Specia | |
| coinControl.destChange = fundDest; | ||
| coinControl.fRequireAllInputs = false; | ||
|
|
||
| for (const auto& out : AvailableCoinsListUnspent(wallet).all()) { | ||
| CTxDestination txDest; | ||
| if (ExtractDestination(out.txout.scriptPubKey, txDest) && txDest == fundDest) { | ||
| coinControl.Select(out.outpoint); | ||
|
|
@@ -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))); | ||
| } | ||
|
|
||
| auto res = CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, coinControl, /*sign=*/true, tx.vExtraPayload.size()); | ||
| if (!res) { | ||
| throw JSONRPCError(RPC_INTERNAL_ERROR, util::ErrorString(res).original); | ||
| } | ||
|
|
||
| const CTransactionRef& newTx = res->tx; | ||
| tx.vin = newTx->vin; | ||
| tx.vout = newTx->vout; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,7 +140,7 @@ static RPCHelpMan masternode_outputs() | |
| CCoinControl coin_control(CoinType::ONLY_MASTERNODE_COLLATERAL); | ||
|
|
||
| UniValue outputsArr(UniValue::VARR); | ||
| for (const auto& out : WITH_LOCK(wallet->cs_wallet, return AvailableCoinsListUnspent(*wallet, &coin_control).all())) { | ||
| outputsArr.push_back(out.outpoint.ToStringShort()); | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,6 +189,28 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker | |
| using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>; | ||
| using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CMutableTransaction>; | ||
|
|
||
| class DeferringSignatureChecker : public BaseSignatureChecker | ||
| { | ||
| protected: | ||
| BaseSignatureChecker& m_checker; | ||
|
|
||
| public: | ||
| DeferringSignatureChecker(BaseSignatureChecker& checker) : m_checker(checker) {} | ||
|
|
||
| bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override | ||
| { | ||
| return m_checker.CheckSig(scriptSig, vchPubKey, scriptCode, sigversion); | ||
| } | ||
| bool CheckLockTime(const CScriptNum& nLockTime) const override | ||
| { | ||
| return m_checker.CheckLockTime(nLockTime); | ||
| } | ||
| bool CheckSequence(const CScriptNum& nSequence) const override | ||
| { | ||
| return m_checker.CheckSequence(nSequence); | ||
| } | ||
| }; | ||
|
|
||
| bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr); | ||
| bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr); | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert should be replaced with proper error handling
The constructor now uses
GetReservedDestination(false)which returns autil::Result. While asserting the result exists works, this could fail in production if the wallet cannot provide addresses (e.g., keypool exhausted). Consider proper error handling instead of assertions.Consider propagating the error instead of asserting:
📝 Committable suggestion
🤖 Prompt for AI Agents