aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/feebumper.cpp10
-rw-r--r--src/wallet/rpc/addresses.cpp1
-rw-r--r--src/wallet/rpc/spend.cpp3
-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.cpp24
-rw-r--r--src/wallet/wallet.h2
10 files changed, 63 insertions, 21 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index f99da926bc..512a011dfc 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -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);
+ CTxDestination dest;
+ ExtractDestination(output.scriptPubKey, dest);
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;
+ 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/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..c2f4321a60 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);
}
}
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 ec7b017720..9d5ce6e125 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -525,6 +525,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 5c297d76e4..dac6e87983 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..3622b00c77 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;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 091a573151..97d06641a7 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;
};