aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/feebumper.cpp24
-rw-r--r--src/wallet/feebumper.h4
-rw-r--r--src/wallet/interfaces.cpp14
-rw-r--r--src/wallet/rpc/addresses.cpp1
-rw-r--r--src/wallet/rpc/spend.cpp21
-rw-r--r--src/wallet/scriptpubkeyman.cpp17
-rw-r--r--src/wallet/scriptpubkeyman.h6
-rw-r--r--src/wallet/spend.cpp17
-rw-r--r--src/wallet/test/spend_tests.cpp2
-rw-r--r--src/wallet/test/wallet_tests.cpp2
-rw-r--r--src/wallet/wallet.cpp26
-rw-r--r--src/wallet/wallet.h3
12 files changed, 99 insertions, 38 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index f99da926bc..f4cb4bbd66 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -162,11 +162,11 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
}
Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
- CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> reduce_output)
+ CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> original_change_index)
{
- // Cannot both specify new outputs and an output to reduce
- if (!outputs.empty() && reduce_output.has_value()) {
- errors.push_back(Untranslated("Cannot specify both new outputs to use and an output index to reduce"));
+ // For now, cannot specify both new outputs to use and an output index to send change
+ if (!outputs.empty() && original_change_index.has_value()) {
+ errors.push_back(Untranslated("The options 'outputs' and 'original_change_index' are incompatible. You can only either specify a new set of outputs, or designate a change output to be recycled."));
return Result::INVALID_PARAMETER;
}
@@ -182,8 +182,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
}
const CWalletTx& wtx = it->second;
- // Make sure that reduce_output is valid
- if (reduce_output.has_value() && reduce_output.value() >= wtx.tx->vout.size()) {
+ // Make sure that original_change_index is valid
+ if (original_change_index.has_value() && original_change_index.value() >= wtx.tx->vout.size()) {
errors.push_back(Untranslated("Change position is out of range"));
return Result::INVALID_PARAMETER;
}
@@ -257,12 +257,12 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
const auto& txouts = outputs.empty() ? wtx.tx->vout : outputs;
for (size_t i = 0; i < txouts.size(); ++i) {
const CTxOut& output = txouts.at(i);
- if (reduce_output.has_value() ? reduce_output.value() == i : OutputIsChange(wallet, output)) {
- CTxDestination change_dest;
- ExtractDestination(output.scriptPubKey, change_dest);
- new_coin_control.destChange = change_dest;
+ CTxDestination dest;
+ ExtractDestination(output.scriptPubKey, dest);
+ if (original_change_index.has_value() ? original_change_index.value() == i : OutputIsChange(wallet, output)) {
+ new_coin_control.destChange = dest;
} else {
- CRecipient recipient = {output.scriptPubKey, output.nValue, false};
+ CRecipient recipient = {dest, output.nValue, false};
recipients.push_back(recipient);
}
new_outputs_value += output.nValue;
@@ -278,7 +278,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
// Add change as recipient with SFFO flag enabled, so fees are deduced from it.
// If the output differs from the original tx output (because the user customized it) a new change output will be created.
- recipients.emplace_back(CRecipient{GetScriptForDestination(new_coin_control.destChange), new_outputs_value, /*fSubtractFeeFromAmount=*/true});
+ recipients.emplace_back(CRecipient{new_coin_control.destChange, new_outputs_value, /*fSubtractFeeFromAmount=*/true});
new_coin_control.destChange = CNoDestination();
}
diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h
index f00bf15730..d3d43861ef 100644
--- a/src/wallet/feebumper.h
+++ b/src/wallet/feebumper.h
@@ -44,7 +44,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
* @param[out] mtx The bump transaction itself
* @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet
* @param[in] outputs Vector of new outputs to replace the bumped transaction's outputs
- * @param[in] reduce_output The position of the change output to deduct the fee from in the transaction being bumped
+ * @param[in] original_change_index The position of the change output to deduct the fee from in the transaction being bumped
*/
Result CreateRateBumpTransaction(CWallet& wallet,
const uint256& txid,
@@ -55,7 +55,7 @@ Result CreateRateBumpTransaction(CWallet& wallet,
CMutableTransaction& mtx,
bool require_mine,
const std::vector<CTxOut>& outputs,
- std::optional<uint32_t> reduce_output = std::nullopt);
+ std::optional<uint32_t> original_change_index = std::nullopt);
//! Sign the new transaction,
//! @return false if the tx couldn't be found or if it was
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 5f2aee6923..65285187f4 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -41,6 +41,7 @@ using interfaces::Wallet;
using interfaces::WalletAddress;
using interfaces::WalletBalances;
using interfaces::WalletLoader;
+using interfaces::WalletMigrationResult;
using interfaces::WalletOrderForm;
using interfaces::WalletTx;
using interfaces::WalletTxOut;
@@ -66,6 +67,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
result.txout_address_is_mine.reserve(wtx.tx->vout.size());
for (const auto& txout : wtx.tx->vout) {
result.txout_is_mine.emplace_back(wallet.IsMine(txout));
+ result.txout_is_change.push_back(OutputIsChange(wallet, txout));
result.txout_address.emplace_back();
result.txout_address_is_mine.emplace_back(ExtractDestination(txout.scriptPubKey, result.txout_address.back()) ?
wallet.IsMine(result.txout_address.back()) :
@@ -630,6 +632,18 @@ public:
return util::Error{error};
}
}
+ util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase) override
+ {
+ auto res = wallet::MigrateLegacyToDescriptor(name, passphrase, m_context);
+ if (!res) return util::Error{util::ErrorString(res)};
+ WalletMigrationResult out{
+ .wallet = MakeWallet(m_context, res->wallet),
+ .watchonly_wallet_name = res->watchonly_wallet ? std::make_optional(res->watchonly_wallet->GetName()) : std::nullopt,
+ .solvables_wallet_name = res->solvables_wallet ? std::make_optional(res->solvables_wallet->GetName()) : std::nullopt,
+ .backup_path = res->backup_path,
+ };
+ return {std::move(out)}; // std::move to work around clang bug
+ }
std::string getWalletDir() override
{
return fs::PathToString(GetWalletDir());
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index c1b99a4f97..e9b93afc30 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -427,6 +427,7 @@ public:
explicit DescribeWalletAddressVisitor(const SigningProvider* _provider) : provider(_provider) {}
UniValue operator()(const CNoDestination& dest) const { return UniValue(UniValue::VOBJ); }
+ UniValue operator()(const PubKeyDestination& dest) const { return UniValue(UniValue::VOBJ); }
UniValue operator()(const PKHash& pkhash) const
{
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index c4206e9897..6b96fc4e49 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -39,7 +39,6 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& sub
}
destinations.insert(dest);
- CScript script_pub_key = GetScriptForDestination(dest);
CAmount amount = AmountFromValue(address_amounts[i++]);
bool subtract_fee = false;
@@ -50,7 +49,7 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& sub
}
}
- CRecipient recipient = {script_pub_key, amount, subtract_fee};
+ CRecipient recipient = {dest, amount, subtract_fee};
recipients.push_back(recipient);
}
}
@@ -1017,10 +1016,14 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
{"outputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "The outputs specified as key-value pairs.\n"
"Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.\n"
"At least one output of either type must be specified.\n"
- "Cannot be provided if 'reduce_output' is specified.",
+ "Cannot be provided if 'original_change_index' is specified.",
OutputsDoc(),
RPCArgOptions{.skip_type_check = true}},
- {"reduce_output", RPCArg::Type::NUM, RPCArg::DefaultHint{"not set, detect change automatically"}, "The 0-based index of the output from which the additional fees will be deducted. In general, this should be the position of change output. Cannot be provided if 'outputs' is specified."},
+ {"original_change_index", RPCArg::Type::NUM, RPCArg::DefaultHint{"not set, detect change automatically"}, "The 0-based index of the change output on the original transaction. "
+ "The indicated output will be recycled into the new change output on the bumped transaction. "
+ "The remainder after paying the recipients and fees will be sent to the output script of the "
+ "original change output. The change output’s amount can increase if bumping the transaction "
+ "adds new inputs, otherwise it will decrease. Cannot be used in combination with the 'outputs' option."},
},
RPCArgOptions{.oneline_description="options"}},
},
@@ -1059,7 +1062,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
coin_control.m_signal_bip125_rbf = true;
std::vector<CTxOut> outputs;
- std::optional<uint32_t> reduce_output;
+ std::optional<uint32_t> original_change_index;
if (!request.params[1].isNull()) {
UniValue options = request.params[1];
@@ -1071,7 +1074,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
{"replaceable", UniValueType(UniValue::VBOOL)},
{"estimate_mode", UniValueType(UniValue::VSTR)},
{"outputs", UniValueType()}, // will be checked by AddOutputs()
- {"reduce_output", UniValueType(UniValue::VNUM)},
+ {"original_change_index", UniValueType(UniValue::VNUM)},
},
true, true);
@@ -1096,8 +1099,8 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
outputs = tempTx.vout;
}
- if (options.exists("reduce_output")) {
- reduce_output = options["reduce_output"].getInt<uint32_t>();
+ if (options.exists("original_change_index")) {
+ original_change_index = options["original_change_index"].getInt<uint32_t>();
}
}
@@ -1116,7 +1119,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
CMutableTransaction mtx;
feebumper::Result res;
// Targeting feerate bump.
- res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, reduce_output);
+ res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, original_change_index);
if (res != feebumper::Result::OK) {
switch(res) {
case feebumper::Result::INVALID_ADDRESS_OR_KEY:
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 1f510d1c58..20f735da12 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -1716,8 +1716,23 @@ std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPub
}
// All watchonly scripts are raw
- spks.insert(setWatchOnly.begin(), setWatchOnly.end());
+ for (const CScript& script : setWatchOnly) {
+ // As the legacy wallet allowed to import any script, we need to verify the validity here.
+ // LegacyScriptPubKeyMan::IsMine() return 'ISMINE_NO' for invalid or not watched scripts (IsMineResult::INVALID or IsMineResult::NO).
+ // e.g. a "sh(sh(pkh()))" which legacy wallets allowed to import!.
+ if (IsMine(script) != ISMINE_NO) spks.insert(script);
+ }
+
+ return spks;
+}
+std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetNotMineScriptPubKeys() const
+{
+ LOCK(cs_KeyStore);
+ std::unordered_set<CScript, SaltedSipHasher> spks;
+ for (const CScript& script : setWatchOnly) {
+ if (IsMine(script) == ISMINE_NO) spks.insert(script);
+ }
return spks;
}
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 3722c1ae1f..7c0eca1475 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -519,6 +519,12 @@ public:
std::set<CKeyID> GetKeys() const override;
std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override;
+ /**
+ * Retrieves scripts that were imported by bugs into the legacy spkm and are
+ * simply invalid, such as a sh(sh(pkh())) script, or not watched.
+ */
+ std::unordered_set<CScript, SaltedSipHasher> GetNotMineScriptPubKeys() const;
+
/** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan.
* Does not modify this ScriptPubKeyMan. */
std::optional<MigrationData> MigrateToDescriptor();
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 96c9a3dc16..7e6fba33aa 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -505,8 +505,15 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
coins_params.skip_locked = false;
for (const COutput& coin : AvailableCoins(wallet, &coin_control, /*feerate=*/std::nullopt, coins_params).All()) {
CTxDestination address;
- if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) &&
- ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
+ if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable))) {
+ if (!ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
+ // For backwards compatibility, we convert P2PK output scripts into PKHash destinations
+ if (auto pk_dest = std::get_if<PubKeyDestination>(&address)) {
+ address = PKHash(pk_dest->GetPubKey());
+ } else {
+ continue;
+ }
+ }
result[address].emplace_back(coin);
}
}
@@ -1071,7 +1078,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// vouts to the payees
for (const auto& recipient : vecSend)
{
- CTxOut txout(recipient.nAmount, recipient.scriptPubKey);
+ CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest));
// Include the fee cost for outputs.
coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION);
@@ -1319,7 +1326,9 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
// Turn the txout set into a CRecipient vector.
for (size_t idx = 0; idx < tx.vout.size(); idx++) {
const CTxOut& txOut = tx.vout[idx];
- CRecipient recipient = {txOut.scriptPubKey, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1};
+ CTxDestination dest;
+ ExtractDestination(txOut.scriptPubKey, dest);
+ CRecipient recipient = {dest, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1};
vecSend.push_back(recipient);
}
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index eca1d74cf6..68c98ae6b9 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
// leftover input amount which would have been change to the recipient
// instead of the miner.
auto check_tx = [&wallet](CAmount leftover_input_amount) {
- CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true};
+ CRecipient recipient{PubKeyDestination({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true};
constexpr int RANDOM_CHANGE_POSITION = -1;
CCoinControl coin_control;
coin_control.m_feerate.emplace(10000);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index ab17f51f8e..21ed52731a 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -645,7 +645,7 @@ void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount
{
LOCK(context.wallet->cs_wallet);
util::Result<CTxDestination> dest = Assert(context.wallet->GetNewDestination(out_type, ""));
- CWalletTx& wtx = context.AddTx(CRecipient{{GetScriptForDestination(*dest)}, amount, /*fSubtractFeeFromAmount=*/true});
+ CWalletTx& wtx = context.AddTx(CRecipient{*dest, amount, /*fSubtractFeeFromAmount=*/true});
CoinFilterParams filter;
filter.skip_locked = false;
CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 6f5248efaf..2459908419 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2213,15 +2213,13 @@ OutputType CWallet::TransactionChangeType(const std::optional<OutputType>& chang
bool any_pkh{false};
for (const auto& recipient : vecSend) {
- std::vector<std::vector<uint8_t>> dummy;
- const TxoutType type{Solver(recipient.scriptPubKey, dummy)};
- if (type == TxoutType::WITNESS_V1_TAPROOT) {
+ if (std::get_if<WitnessV1Taproot>(&recipient.dest)) {
any_tr = true;
- } else if (type == TxoutType::WITNESS_V0_KEYHASH) {
+ } else if (std::get_if<WitnessV0KeyHash>(&recipient.dest)) {
any_wpkh = true;
- } else if (type == TxoutType::SCRIPTHASH) {
+ } else if (std::get_if<ScriptHash>(&recipient.dest)) {
any_sh = true;
- } else if (type == TxoutType::PUBKEYHASH) {
+ } else if (std::get_if<PKHash>(&recipient.dest)) {
any_pkh = true;
}
}
@@ -3862,6 +3860,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
return false;
}
+ // Get all invalid or non-watched scripts that will not be migrated
+ std::set<CTxDestination> not_migrated_dests;
+ for (const auto& script : legacy_spkm->GetNotMineScriptPubKeys()) {
+ CTxDestination dest;
+ if (ExtractDestination(script, dest)) not_migrated_dests.emplace(dest);
+ }
+
for (auto& desc_spkm : data.desc_spkms) {
if (m_spk_managers.count(desc_spkm->GetID()) > 0) {
error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted.");
@@ -3968,6 +3973,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
continue;
}
}
+
+ // Skip invalid/non-watched scripts that will not be migrated
+ if (not_migrated_dests.count(addr_pair.first) > 0) {
+ dests_to_delete.push_back(addr_pair.first);
+ continue;
+ }
+
// Not ours, not in watchonly wallet, and not in solvable
error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets");
return false;
@@ -4201,7 +4213,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
// Migration successful, unload the wallet locally, then reload it.
assert(local_wallet.use_count() == 1);
local_wallet.reset();
- LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
+ res.wallet = LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
res.wallet_name = wallet_name;
} else {
// Migration failed, cleanup
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 091a573151..5adb8b6e27 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -288,7 +288,7 @@ inline std::optional<AddressPurpose> PurposeFromString(std::string_view s)
struct CRecipient
{
- CScript scriptPubKey;
+ CTxDestination dest;
CAmount nAmount;
bool fSubtractFeeFromAmount;
};
@@ -1087,6 +1087,7 @@ bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_nam
struct MigrationResult {
std::string wallet_name;
+ std::shared_ptr<CWallet> wallet;
std::shared_ptr<CWallet> watchonly_wallet;
std::shared_ptr<CWallet> solvables_wallet;
fs::path backup_path;
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-31 18:36:03 +0000